fix bgworkers in EXEC_BACKEND

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: fix bgworkers in EXEC_BACKEND
Date: 2012-12-27 17:15:27
Message-ID: 20121227171527.GB4238@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
maxbackend-macro.patch text/x-diff 10.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2012-12-27 17:22:56 Re: fix bgworkers in EXEC_BACKEND
Previous Message Tom Lane 2012-12-27 16:33:55 Re: buffer assertion tripping under repeat pgbench load