Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e880df2

Browse files
committed
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
1 parent 86e640a commit e880df2

File tree

1 file changed

+22
-17
lines changed

1 file changed

+22
-17
lines changed

src/backend/postmaster/postmaster.c

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ static void TerminateChildren(int signal);
413413

414414
static int CountChildren(int target);
415415
static bool assign_backendlist_entry(RegisteredBgWorker *rw);
416-
static void maybe_start_bgworker(void);
416+
static void maybe_start_bgworkers(void);
417417
static bool CreateOptsFile(int argc, char *argv[], char *fullprogname);
418418
static pid_t StartChildProcess(AuxProcType type);
419419
static void StartAutovacuumWorker(void);
@@ -1318,7 +1318,7 @@ PostmasterMain(int argc, char *argv[])
13181318
pmState = PM_STARTUP;
13191319

13201320
/* Some workers may be scheduled to start now */
1321-
maybe_start_bgworker();
1321+
maybe_start_bgworkers();
13221322

13231323
status = ServerLoop();
13241324

@@ -1785,7 +1785,7 @@ ServerLoop(void)
17851785

17861786
/* Get other worker processes running, if needed */
17871787
if (StartWorkerNeeded || HaveCrashedWorker)
1788-
maybe_start_bgworker();
1788+
maybe_start_bgworkers();
17891789

17901790
#ifdef HAVE_PTHREAD_IS_THREADED_NP
17911791

@@ -2823,7 +2823,7 @@ reaper(SIGNAL_ARGS)
28232823
PgStatPID = pgstat_start();
28242824

28252825
/* workers may be scheduled to start now */
2826-
maybe_start_bgworker();
2826+
maybe_start_bgworkers();
28272827

28282828
/* at this point we are really open for business */
28292829
ereport(LOG,
@@ -4970,7 +4970,7 @@ sigusr1_handler(SIGNAL_ARGS)
49704970
}
49714971

49724972
if (StartWorkerNeeded || HaveCrashedWorker)
4973-
maybe_start_bgworker();
4973+
maybe_start_bgworkers();
49744974

49754975
if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) &&
49764976
PgArchPID != 0)
@@ -5679,22 +5679,23 @@ assign_backendlist_entry(RegisteredBgWorker *rw)
56795679
}
56805680

56815681
/*
5682-
* If the time is right, start one background worker.
5682+
* If the time is right, start background worker(s).
56835683
*
5684-
* As a side effect, the bgworker control variables are set or reset whenever
5685-
* there are more workers to start after this one, and whenever the overall
5686-
* system state requires it.
5684+
* As a side effect, the bgworker control variables are set or reset
5685+
* depending on whether more workers may need to be started.
56875686
*
5688-
* The reason we start at most one worker per call is to avoid consuming the
5687+
* We limit the number of workers started per call, to avoid consuming the
56895688
* postmaster's attention for too long when many such requests are pending.
56905689
* As long as StartWorkerNeeded is true, ServerLoop will not block and will
56915690
* call this function again after dealing with any other issues.
56925691
*/
56935692
static void
5694-
maybe_start_bgworker(void)
5693+
maybe_start_bgworkers(void)
56955694
{
5696-
slist_mutable_iter iter;
5695+
#define MAX_BGWORKERS_TO_LAUNCH 100
5696+
int num_launched = 0;
56975697
TimestampTz now = 0;
5698+
slist_mutable_iter iter;
56985699

56995700
/*
57005701
* During crash recovery, we have no need to be called until the state
@@ -5779,12 +5780,16 @@ maybe_start_bgworker(void)
57795780
}
57805781

57815782
/*
5782-
* Quit, but have ServerLoop call us again to look for additional
5783-
* ready-to-run workers. There might not be any, but we'll find
5784-
* out the next time we run.
5783+
* If we've launched as many workers as allowed, quit, but have
5784+
* ServerLoop call us again to look for additional ready-to-run
5785+
* workers. There might not be any, but we'll find out the next
5786+
* time we run.
57855787
*/
5786-
StartWorkerNeeded = true;
5787-
return;
5788+
if (++num_launched >= MAX_BGWORKERS_TO_LAUNCH)
5789+
{
5790+
StartWorkerNeeded = true;
5791+
return;
5792+
}
57885793
}
57895794
}
57905795
}

0 commit comments

Comments
 (0)