Skip to content

Commit b8a439b

Browse files
robertmhaasrobertmhaas
committed
Allow notifications to bgworkers without database connections.
Previously, if one background worker registered another background worker and set bgw_notify_pid while for the second background worker, it would not receive notifications from the postmaster unless, at the time the "parent" was registered, BGWORKER_BACKEND_DATABASE_CONNECTION was set. To fix, instead instead of including only those background workers that requested database connections in the postmater's BackendList, include them all. There doesn't seem to be any reason not do this, and indeed it removes a significant amount of duplicated code. The other option is to make PostmasterMarkPIDForWorkerNotify look at BackgroundWorkerList in addition to BackendList, but that adds more code duplication instead of getting rid of it. Patch by me. Review and testing by Ashutosh Bapat.
1 parent c1564b3 commit b8a439b

File tree

1 file changed

+25
-111
lines changed

1 file changed

+25
-111
lines changed

‎src/backend/postmaster/postmaster.c

+25-111
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,7 @@
155155
* they will never become live backends. dead_end children are not assigned a
156156
* PMChildSlot.
157157
*
158-
* Background workers that request shared memory access during registration are
159-
* in this list, too.
158+
* Background workers are in this list, too.
160159
*/
161160
typedef struct bkend
162161
{
@@ -404,13 +403,11 @@ static long PostmasterRandom(void);
404403
static void RandomSalt(char *md5Salt);
405404
static void signal_child(pid_t pid, int signal);
406405
static bool SignalSomeChildren(int signal, int targets);
407-
static bool SignalUnconnectedWorkers(int signal);
408406
static void TerminateChildren(int signal);
409407

410408
#define SignalChildren(sig) SignalSomeChildren(sig, BACKEND_TYPE_ALL)
411409

412410
static int CountChildren(int target);
413-
static int CountUnconnectedWorkers(void);
414411
static void maybe_start_bgworker(void);
415412
static bool CreateOptsFile(int argc, char *argv[], char *fullprogname);
416413
static pid_t StartChildProcess(AuxProcType type);
@@ -2414,7 +2411,6 @@ SIGHUP_handler(SIGNAL_ARGS)
24142411
(errmsg("received SIGHUP, reloading configuration files")));
24152412
ProcessConfigFile(PGC_SIGHUP);
24162413
SignalChildren(SIGHUP);
2417-
SignalUnconnectedWorkers(SIGHUP);
24182414
if (StartupPID != 0)
24192415
signal_child(StartupPID, SIGHUP);
24202416
if (BgWriterPID != 0)
@@ -2491,7 +2487,6 @@ pmdie(SIGNAL_ARGS)
24912487
/* and bgworkers too; does this need tweaking? */
24922488
SignalSomeChildren(SIGTERM,
24932489
BACKEND_TYPE_AUTOVAC | BACKEND_TYPE_BGWORKER);
2494-
SignalUnconnectedWorkers(SIGTERM);
24952490
/* and the autovac launcher too */
24962491
if (AutoVacPID != 0)
24972492
signal_child(AutoVacPID, SIGTERM);
@@ -2543,11 +2538,11 @@ pmdie(SIGNAL_ARGS)
25432538
signal_child(BgWriterPID, SIGTERM);
25442539
if (WalReceiverPID != 0)
25452540
signal_child(WalReceiverPID, SIGTERM);
2546-
SignalUnconnectedWorkers(SIGTERM);
25472541
if (pmState == PM_RECOVERY)
25482542
{
2543+
SignalSomeChildren(SIGTERM, BACKEND_TYPE_BGWORKER);
25492544
/*
2550-
* Only startup, bgwriter, walreceiver, unconnected bgworkers,
2545+
* Only startup, bgwriter, walreceiver, possibly bgworkers,
25512546
* and/or checkpointer should be active in this state; we just
25522547
* signaled the first four, and we don't want to kill
25532548
* checkpointer yet.
@@ -2999,25 +2994,21 @@ CleanupBackgroundWorker(int pid,
29992994
}
30002995

30012996
/* Get it out of the BackendList and clear out remaining data */
3002-
if (rw->rw_backend)
3003-
{
3004-
Assert(rw->rw_worker.bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION);
3005-
dlist_delete(&rw->rw_backend->elem);
2997+
dlist_delete(&rw->rw_backend->elem);
30062998
#ifdef EXEC_BACKEND
3007-
ShmemBackendArrayRemove(rw->rw_backend);
2999+
ShmemBackendArrayRemove(rw->rw_backend);
30083000
#endif
30093001

3010-
/*
3011-
* It's possible that this background worker started some OTHER
3012-
* background worker and asked to be notified when that worker
3013-
* started or stopped. If so, cancel any notifications destined
3014-
* for the now-dead backend.
3015-
*/
3016-
if (rw->rw_backend->bgworker_notify)
3017-
BackgroundWorkerStopNotifications(rw->rw_pid);
3018-
free(rw->rw_backend);
3019-
rw->rw_backend = NULL;
3020-
}
3002+
/*
3003+
* It's possible that this background worker started some OTHER
3004+
* background worker and asked to be notified when that worker
3005+
* started or stopped. If so, cancel any notifications destined
3006+
* for the now-dead backend.
3007+
*/
3008+
if (rw->rw_backend->bgworker_notify)
3009+
BackgroundWorkerStopNotifications(rw->rw_pid);
3010+
free(rw->rw_backend);
3011+
rw->rw_backend = NULL;
30213012
rw->rw_pid = 0;
30223013
rw->rw_child_slot = 0;
30233014
ReportBackgroundWorkerPID(rw); /* report child death */
@@ -3160,15 +3151,12 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
31603151
* Found entry for freshly-dead worker, so remove it.
31613152
*/
31623153
(void) ReleasePostmasterChildSlot(rw->rw_child_slot);
3163-
if (rw->rw_backend)
3164-
{
3165-
dlist_delete(&rw->rw_backend->elem);
3154+
dlist_delete(&rw->rw_backend->elem);
31663155
#ifdef EXEC_BACKEND
3167-
ShmemBackendArrayRemove(rw->rw_backend);
3156+
ShmemBackendArrayRemove(rw->rw_backend);
31683157
#endif
3169-
free(rw->rw_backend);
3170-
rw->rw_backend = NULL;
3171-
}
3158+
free(rw->rw_backend);
3159+
rw->rw_backend = NULL;
31723160
rw->rw_pid = 0;
31733161
rw->rw_child_slot = 0;
31743162
/* don't reset crashed_at */
@@ -3505,7 +3493,6 @@ PostmasterStateMachine(void)
35053493
* process.
35063494
*/
35073495
if (CountChildren(BACKEND_TYPE_NORMAL | BACKEND_TYPE_WORKER) == 0 &&
3508-
CountUnconnectedWorkers() == 0 &&
35093496
StartupPID == 0 &&
35103497
WalReceiverPID == 0 &&
35113498
BgWriterPID == 0 &&
@@ -3727,39 +3714,6 @@ signal_child(pid_t pid, int signal)
37273714
#endif
37283715
}
37293716

3730-
/*
3731-
* Send a signal to bgworkers that did not request backend connections
3732-
*
3733-
* The reason this is interesting is that workers that did request connections
3734-
* are considered by SignalChildren; this function complements that one.
3735-
*/
3736-
static bool
3737-
SignalUnconnectedWorkers(int signal)
3738-
{
3739-
slist_iter iter;
3740-
bool signaled = false;
3741-
3742-
slist_foreach(iter, &BackgroundWorkerList)
3743-
{
3744-
RegisteredBgWorker *rw;
3745-
3746-
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur);
3747-
3748-
if (rw->rw_pid == 0)
3749-
continue;
3750-
/* ignore connected workers */
3751-
if (rw->rw_backend != NULL)
3752-
continue;
3753-
3754-
ereport(DEBUG4,
3755-
(errmsg_internal("sending signal %d to process %d",
3756-
signal, (int) rw->rw_pid)));
3757-
signal_child(rw->rw_pid, signal);
3758-
signaled = true;
3759-
}
3760-
return signaled;
3761-
}
3762-
37633717
/*
37643718
* Send a signal to the targeted children (but NOT special children;
37653719
* dead_end children are never signaled, either).
@@ -3832,7 +3786,6 @@ TerminateChildren(int signal)
38323786
signal_child(PgArchPID, signal);
38333787
if (PgStatPID != 0)
38343788
signal_child(PgStatPID, signal);
3835-
SignalUnconnectedWorkers(signal);
38363789
}
38373790

38383791
/*
@@ -5093,33 +5046,6 @@ PostmasterRandom(void)
50935046
return random();
50945047
}
50955048

5096-
/*
5097-
* Count up number of worker processes that did not request backend connections
5098-
* See SignalUnconnectedWorkers for why this is interesting.
5099-
*/
5100-
static int
5101-
CountUnconnectedWorkers(void)
5102-
{
5103-
slist_iter iter;
5104-
int cnt = 0;
5105-
5106-
slist_foreach(iter, &BackgroundWorkerList)
5107-
{
5108-
RegisteredBgWorker *rw;
5109-
5110-
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur);
5111-
5112-
if (rw->rw_pid == 0)
5113-
continue;
5114-
/* ignore connected workers */
5115-
if (rw->rw_backend != NULL)
5116-
continue;
5117-
5118-
cnt++;
5119-
}
5120-
return cnt;
5121-
}
5122-
51235049
/*
51245050
* Count up number of child processes of specified types (dead_end chidren
51255051
* are always excluded).
@@ -5520,8 +5446,7 @@ do_start_bgworker(RegisteredBgWorker *rw)
55205446
#endif
55215447
default:
55225448
rw->rw_pid = worker_pid;
5523-
if (rw->rw_backend)
5524-
rw->rw_backend->pid = rw->rw_pid;
5449+
rw->rw_backend->pid = rw->rw_pid;
55255450
ReportBackgroundWorkerPID(rw);
55265451
}
55275452
}
@@ -5684,30 +5609,19 @@ maybe_start_bgworker(void)
56845609
rw->rw_crashed_at = 0;
56855610

56865611
/*
5687-
* If necessary, allocate and assign the Backend element. Note we
5612+
* Allocate and assign the Backend element. Note we
56885613
* must do this before forking, so that we can handle out of
56895614
* memory properly.
5690-
*
5691-
* If not connected, we don't need a Backend element, but we still
5692-
* need a PMChildSlot.
56935615
*/
5694-
if (rw->rw_worker.bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION)
5695-
{
5696-
if (!assign_backendlist_entry(rw))
5697-
return;
5698-
}
5699-
else
5700-
rw->rw_child_slot = MyPMChildSlot = AssignPostmasterChildSlot();
5616+
if (!assign_backendlist_entry(rw))
5617+
return;
57015618

57025619
do_start_bgworker(rw); /* sets rw->rw_pid */
57035620

5704-
if (rw->rw_backend)
5705-
{
5706-
dlist_push_head(&BackendList, &rw->rw_backend->elem);
5621+
dlist_push_head(&BackendList, &rw->rw_backend->elem);
57075622
#ifdef EXEC_BACKEND
5708-
ShmemBackendArrayAdd(rw->rw_backend);
5623+
ShmemBackendArrayAdd(rw->rw_backend);
57095624
#endif
5710-
}
57115625

57125626
/*
57135627
* Have ServerLoop call us again. Note that there might not

0 commit comments

Comments
 (0)