fix bgworkers in EXEC_BACKEND - Mailing list pgsql-hackers

From Alvaro Herrera
Subject fix bgworkers in EXEC_BACKEND
Date
Msg-id 20121227171527.GB4238@alvh.no-ip.org
Whole thread Raw
Responses Re: fix bgworkers in EXEC_BACKEND
Re: fix bgworkers in EXEC_BACKEND
List pgsql-hackers
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.

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'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.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: buffer assertion tripping under repeat pgbench load
Next
From: Magnus Hagander
Date:
Subject: Re: fix bgworkers in EXEC_BACKEND