Allow multiple bgworkers to be launched per postmaster iteration.

Previously, maybe_start_bgworker() would launch at most one bgworker
process per call, on the grounds that the postmaster might otherwise
neglect its other duties for too long.  However, that seems overly
conservative, especially since bad effects only become obvious when
many hundreds of bgworkers need to be launched at once.  On the other
side of the coin is that the existing logic could result in substantial
delay of bgworker launches, because ServerLoop isn't guaranteed to
iterate immediately after a signal arrives.  (My attempt to fix that
by using pselect(2) encountered too many portability question marks,
and in any case could not help on platforms without pselect().)
One could also question the wisdom of using an O(N^2) processing
method if the system is intended to support so many bgworkers.

As a compromise, allow that function to launch up to 100 bgworkers
per call (and in consequence, rename it to maybe_start_bgworkers).
This will allow any normal parallel-query request for workers
to be satisfied immediately during sigusr1_handler, avoiding the
question of whether ServerLoop will be able to launch more promptly.

There is talk of rewriting the postmaster to use a WaitEventSet to
avoid the signal-response-delay problem, but I'd argue that this change
should be kept even after that happens (if it ever does).

Backpatch to 9.6 where parallel query was added.  The issue exists
before that, but previous uses of bgworkers typically aren't as
sensitive to how quickly they get launched.

Discussion: https://postgr.es/m/4707.1493221358@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2017-04-26 16:17:29 -04:00
parent fda4fec578
commit aa1351f1ee
1 changed files with 22 additions and 17 deletions

View File

@ -421,7 +421,7 @@ static void TerminateChildren(int signal);
static int CountChildren(int target);
static bool assign_backendlist_entry(RegisteredBgWorker *rw);
static void maybe_start_bgworker(void);
static void maybe_start_bgworkers(void);
static bool CreateOptsFile(int argc, char *argv[], char *fullprogname);
static pid_t StartChildProcess(AuxProcType type);
static void StartAutovacuumWorker(void);
@ -1346,7 +1346,7 @@ PostmasterMain(int argc, char *argv[])
pmState = PM_STARTUP;
/* Some workers may be scheduled to start now */
maybe_start_bgworker();
maybe_start_bgworkers();
status = ServerLoop();
@ -1813,7 +1813,7 @@ ServerLoop(void)
/* Get other worker processes running, if needed */
if (StartWorkerNeeded || HaveCrashedWorker)
maybe_start_bgworker();
maybe_start_bgworkers();
#ifdef HAVE_PTHREAD_IS_THREADED_NP
@ -2859,7 +2859,7 @@ reaper(SIGNAL_ARGS)
PgStatPID = pgstat_start();
/* workers may be scheduled to start now */
maybe_start_bgworker();
maybe_start_bgworkers();
/* at this point we are really open for business */
ereport(LOG,
@ -5026,7 +5026,7 @@ sigusr1_handler(SIGNAL_ARGS)
}
if (StartWorkerNeeded || HaveCrashedWorker)
maybe_start_bgworker();
maybe_start_bgworkers();
if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) &&
PgArchPID != 0)
@ -5726,22 +5726,23 @@ assign_backendlist_entry(RegisteredBgWorker *rw)
}
/*
* If the time is right, start one background worker.
* If the time is right, start background worker(s).
*
* As a side effect, the bgworker control variables are set or reset whenever
* there are more workers to start after this one, and whenever the overall
* system state requires it.
* As a side effect, the bgworker control variables are set or reset
* depending on whether more workers may need to be started.
*
* The reason we start at most one worker per call is to avoid consuming the
* We limit the number of workers started per call, to avoid consuming the
* postmaster's attention for too long when many such requests are pending.
* As long as StartWorkerNeeded is true, ServerLoop will not block and will
* call this function again after dealing with any other issues.
*/
static void
maybe_start_bgworker(void)
maybe_start_bgworkers(void)
{
slist_mutable_iter iter;
#define MAX_BGWORKERS_TO_LAUNCH 100
int num_launched = 0;
TimestampTz now = 0;
slist_mutable_iter iter;
/*
* During crash recovery, we have no need to be called until the state
@ -5826,12 +5827,16 @@ maybe_start_bgworker(void)
}
/*
* Quit, but have ServerLoop call us again to look for additional
* ready-to-run workers. There might not be any, but we'll find
* out the next time we run.
* If we've launched as many workers as allowed, quit, but have
* ServerLoop call us again to look for additional ready-to-run
* workers. There might not be any, but we'll find out the next
* time we run.
*/
StartWorkerNeeded = true;
return;
if (++num_launched >= MAX_BGWORKERS_TO_LAUNCH)
{
StartWorkerNeeded = true;
return;
}
}
}
}