On Thu, Dec 27, 2012 at 6:15 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> 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.
Isn't that version going to make the source field in pg_settings for max_connections show override whenever you are using background workers? Thus breaking things like the fields to tell you which config file and line it's on? > 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. That seems neater. > 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'd like to hear opinions on just staying with the IMO ugly minimal fix, > or pursue instead making MaxBackends a macro plus some sort of cache to > avoid repeated computation. If my understanding per above is correct, then here's a +1 for the non-ugly non-minimal fix. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers