Eliminate one background-worker-related flag variable.

robertmhaas · robertmhaas · commit c6fda5a19f7d · 2014-10-04T22:15:06.000-04:00
Teach sigusr1_handler() to use the same test for whether a worker
might need to be started as ServerLoop().  Aside from being perhaps
a bit simpler, this prevents a potentially-unbounded delay when
starting a background worker.  On some platforms, select() doesn't
return when interrupted by a signal, but is instead restarted,
including a reset of the timeout to the originally-requested value.
If signals arrive often enough, but no connection requests arrive,
sigusr1_handler() will be executed repeatedly, but the body of
ServerLoop() won't be reached.  This change ensures that, even in
that case, background workers will eventually get launched.

This is far from a perfect fix; really, we need select() to return
control to ServerLoop() after an interrupt, either via the self-pipe
trick or some other mechanism.  But that's going to require more
work and discussion, so let's do this for now to at least mitigate
the damage.

Per investigation of test_shm_mq failures on buildfarm member anole.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
@@ -4757,15 +4757,14 @@ static void
 sigusr1_handler(SIGNAL_ARGS)
 {
 	int			save_errno = errno;
-	bool		start_bgworker = false;
 
 	PG_SETMASK(&BlockSig);
 
 	/* Process background worker state change. */
 	if (CheckPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE))
 	{
 		BackgroundWorkerStateChange();
-		start_bgworker = true;
+		StartWorkerNeeded = true;
 	}
 
 	/*
@@ -4806,10 +4805,10 @@ sigusr1_handler(SIGNAL_ARGS)
 
 		pmState = PM_HOT_STANDBY;
 		/* Some workers may be scheduled to start now */
-		start_bgworker = true;
+		StartWorkerNeeded = true;
 	}
 
-	if (start_bgworker)
+	if (StartWorkerNeeded || HaveCrashedWorker)
 		maybe_start_bgworker();
 
 	if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) &&

Original file line number	Diff line number	Diff line change
`@@ -4757,15 +4757,14 @@ static void`
`4757`	`4757`	`sigusr1_handler(SIGNAL_ARGS)`
`4758`	`4758`	`{`
`4759`	`4759`	`int save_errno = errno;`
`4760`		`- bool start_bgworker = false;`
`4761`	`4760`
`4762`	`4761`	`PG_SETMASK(&BlockSig);`
`4763`	`4762`
`4764`	`4763`	`/* Process background worker state change. */`
`4765`	`4764`	`if (CheckPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE))`
`4766`	`4765`	`{`
`4767`	`4766`	`BackgroundWorkerStateChange();`
`4768`		`- start_bgworker = true;`
	`4767`	`+ StartWorkerNeeded = true;`
`4769`	`4768`	`}`
`4770`	`4769`
`4771`	`4770`	`/*`
`@@ -4806,10 +4805,10 @@ sigusr1_handler(SIGNAL_ARGS)`
`4806`	`4805`
`4807`	`4806`	`pmState = PM_HOT_STANDBY;`
`4808`	`4807`	`/* Some workers may be scheduled to start now */`
`4809`		`- start_bgworker = true;`
	`4808`	`+ StartWorkerNeeded = true;`
`4810`	`4809`	`}`
`4811`	`4810`
`4812`		`- if (start_bgworker)`
	`4811`	`+ if (StartWorkerNeeded \|\| HaveCrashedWorker)`
`4813`	`4812`	`maybe_start_bgworker();`
`4814`	`4813`
`4815`	`4814`	`if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) &&`