Missed check for too-many-children in bgworker spawning
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2019-10-06T17:17:37Z
Lists: pgsql-hackers
Attachments
- check-for-enough-child-slots-for-new-bgworkers.patch (text/x-diff) patch
Over in [1] we have a report of a postmaster shutdown that seems to
have occurred because some client logic was overaggressively spawning
connection requests, causing the postmaster's child-process arrays to
be temporarily full, and then some parallel query tried to launch a
new bgworker process. The postmaster's bgworker-spawning logic lacks
any check for the arrays being full, so when AssignPostmasterChildSlot
failed to find a free slot, kaboom!
The attached proposed patch fixes this by making bgworker spawning
include a canAcceptConnections() test. That's perhaps overkill, since
we really just need to check the CountChildren() total; but I judged
that going through that function and having it decide what to test or
not test was a better design than duplicating the CountChildren() test
elsewhere.
I'd first imagined also replacing the one-size-fits-all check
if (CountChildren(BACKEND_TYPE_ALL) >= MaxLivePostmasterChildren())
result = CAC_TOOMANY;
with something like
switch (backend_type)
{
case BACKEND_TYPE_NORMAL:
if (CountChildren(backend_type) >= 2 * MaxConnections)
result = CAC_TOOMANY;
break;
case BACKEND_TYPE_AUTOVAC:
if (CountChildren(backend_type) >= 2 * autovacuum_max_workers)
result = CAC_TOOMANY;
break;
...
}
so as to subdivide the pool of child-process slots and prevent client
requests from consuming slots meant for background processes. But on
closer examination that's not really worth the trouble, because this
pool is already considerably bigger than MaxBackends; so even if we
prevented a failure here we could still have bgworker startup failure
later on when it tries to acquire a PGPROC.
Barring objections, I'll apply and back-patch this soon.
regards, tom lane
[1] https://www.postgresql.org/message-id/flat/CADCf-WNZk_9680Q0YjfBzuiR0Oe8LzvDs2Ts3_tq6Tv1e8raQQ%40mail.gmail.com
Commits
-
Check for too many postmaster children before spawning a bgworker.
- 8c2910ce5ed7 9.5.20 landed
- c69e982a60fb 9.6.16 landed
- 7e8d0eb63fbb 12.1 landed
- 3887e9455f81 13.0 landed
- 1b5c2ddcdefc 10.11 landed
- 021065aac676 11.6 landed
-
Report an ERROR if a parallel worker fails to start properly.
- 2badb5afb89c 11.0 cited