Re: fix bgworkers in EXEC_BACKEND
От | Heikki Linnakangas |
---|---|
Тема | Re: fix bgworkers in EXEC_BACKEND |
Дата | |
Msg-id | 50DC8E0F.6040609@vmware.com обсуждение исходный текст |
Ответ на | fix bgworkers in EXEC_BACKEND (Alvaro Herrera <alvherre@2ndquadrant.com>) |
Ответы |
Re: fix bgworkers in EXEC_BACKEND
Re: fix bgworkers in EXEC_BACKEND Re: fix bgworkers in EXEC_BACKEND Re: fix bgworkers in EXEC_BACKEND |
Список | pgsql-hackers |
On 27.12.2012 19:15, Alvaro Herrera wrote: > I committed background workers three weeks ago, claiming it worked on > EXEC_BACKEND, and shortly thereafter I discovered that it didn't. I > noticed that the problem is the kludge to cause postmaster and children > to recompute MaxBackends after shared_preload_libraries is processed; so > the minimal fix is to duplicate this bit, from PostmasterMain() into > SubPostmasterMain(): > > @@ -4443,6 +4443,17 @@ SubPostmasterMain(int argc, char *argv[]) > */ > process_shared_preload_libraries(); > > + /* > + * If loadable modules have added background workers, MaxBackends needs to > + * be updated. Do so now by forcing a no-op update of max_connections. > + * XXX This is a pretty ugly way to do it, but it doesn't seem worth > + * introducing a new entry point in guc.c to do it in a cleaner fashion. > + */ > + if (GetNumShmemAttachedBgworkers()> 0) > + SetConfigOption("max_connections", > + GetConfigOption("max_connections", false, false), > + PGC_POSTMASTER, PGC_S_OVERRIDE); > > I considered this pretty ugly when I first wrote it, and as the comment > says I tried to add something to guc.c to make it cleaner, but it was > even uglier. Might be cleaner to directly assign the correct value to MaxBackends above, ie. "MaxBackends = MaxConnections + newval + 1 + GetNumShmemAttachedBgworkers()". With a comment to remind that it needs to be kept in sync with the other places where that calculation is done, in guc.c. Or put that calculation in a new function and call it above and in guc.c. Thinking about this some more, it might be cleaner to move the responsibility of setting MaxBackends out of guc.c, into postmaster.c. The guc machinery would set max_connections and autovacuum_max_workers as usual, but not try to set MaxBackends. After reading the config file in postmaster.c, calculate MaxBackends. This would have the advantage that MaxBackends would be kept set at zero, until we know the final value. That way it's obvious that you cannot trust the value of MaxBackends in a contrib module preload-function, for example, which would reduce the chance of programmer mistakes. > So I now came up with a completely different idea: how about making > MaxBackends a macro, i.e. > > +#define MaxBackends (MaxConnections + autovacuum_max_workers + 1 + \ > + GetNumShmemAttachedBgworkers()) > > so that instead of having guc.c recompute it, each caller that needs to > value obtains it up to date all the time? This additionally means that > assign_maxconnections and assign_autovacuum_max_workers go away (only > the check routines remain). Patch attached. > > The one problem I see as serious with this approach is that it'd be > moderately expensive (i.e. not just fetch a value from memory) to > compute the value because it requires a walk of the registered workers > list. For most callers this wouldn't be a problem because it's just > during shmem sizing/creation; but there are places such as multixact.c > and async.c that use it routinely, so it's likely that we need to cache > the value somehow. It seems relatively straightforward though. I don't like that. The result of GetNumShmemAttachedBgWorkers() doesn't change after postmaster startup, so it seems silly to call it repeatedly. And from a readability point of view, it makes you think that it might change, because it's recalculated every time. If I'm reading the code correctly, GetNumShmemAttachedBgWorkers() works by walking through a backend-local list. What happens if a background worker fails to register itself when preloaded in one backend? That backend would calculate a different value of MaxBackends, with interesting consequences. That would be a clear case of "don't do that", but nevertheless, I think it would be better if we didn't rely on that. I'd suggest adding MaxBackends to the list of variables passed from postmaster to backends via BackendParameters. All in all, I propose the attached. Not tested on Windows. - Heikki
Вложения
В списке pgsql-hackers по дате отправления: