Make BackgroundWorkerList doubly-linked

This allows ForgetBackgroundWorker() and ReportBackgroundWorkerExit()
to take a RegisteredBgWorker pointer as argument, rather than a list
iterator. That feels a little more natural. But more importantly, this
paves the way for more refactoring in the next commit.

Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/835232c0-a5f7-4f20-b95b-5b56ba57d741@iki.fi
This commit is contained in:
Heikki Linnakangas 2024-08-09 22:44:20 +03:00
parent 7fceb5725b
commit b43100fa71
3 changed files with 54 additions and 58 deletions

View File

@ -37,7 +37,7 @@
/* /*
* The postmaster's list of registered background workers, in private memory. * The postmaster's list of registered background workers, in private memory.
*/ */
slist_head BackgroundWorkerList = SLIST_STATIC_INIT(BackgroundWorkerList); dlist_head BackgroundWorkerList = DLIST_STATIC_INIT(BackgroundWorkerList);
/* /*
* BackgroundWorkerSlots exist in shared memory and can be accessed (via * BackgroundWorkerSlots exist in shared memory and can be accessed (via
@ -168,7 +168,7 @@ BackgroundWorkerShmemInit(void)
&found); &found);
if (!IsUnderPostmaster) if (!IsUnderPostmaster)
{ {
slist_iter siter; dlist_iter iter;
int slotno = 0; int slotno = 0;
BackgroundWorkerData->total_slots = max_worker_processes; BackgroundWorkerData->total_slots = max_worker_processes;
@ -181,12 +181,12 @@ BackgroundWorkerShmemInit(void)
* correspondence between the postmaster's private list and the array * correspondence between the postmaster's private list and the array
* in shared memory. * in shared memory.
*/ */
slist_foreach(siter, &BackgroundWorkerList) dlist_foreach(iter, &BackgroundWorkerList)
{ {
BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno]; BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
Assert(slotno < max_worker_processes); Assert(slotno < max_worker_processes);
slot->in_use = true; slot->in_use = true;
slot->terminate = false; slot->terminate = false;
@ -220,13 +220,13 @@ BackgroundWorkerShmemInit(void)
static RegisteredBgWorker * static RegisteredBgWorker *
FindRegisteredWorkerBySlotNumber(int slotno) FindRegisteredWorkerBySlotNumber(int slotno)
{ {
slist_iter siter; dlist_iter iter;
slist_foreach(siter, &BackgroundWorkerList) dlist_foreach(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_shmem_slot == slotno) if (rw->rw_shmem_slot == slotno)
return rw; return rw;
} }
@ -413,29 +413,25 @@ BackgroundWorkerStateChange(bool allow_new_workers)
(errmsg_internal("registering background worker \"%s\"", (errmsg_internal("registering background worker \"%s\"",
rw->rw_worker.bgw_name))); rw->rw_worker.bgw_name)));
slist_push_head(&BackgroundWorkerList, &rw->rw_lnode); dlist_push_head(&BackgroundWorkerList, &rw->rw_lnode);
} }
} }
/* /*
* Forget about a background worker that's no longer needed. * Forget about a background worker that's no longer needed.
* *
* The worker must be identified by passing an slist_mutable_iter that * NOTE: The entry is unlinked from BackgroundWorkerList. If the caller is
* points to it. This convention allows deletion of workers during * iterating through it, better use a mutable iterator!
* searches of the worker list, and saves having to search the list again.
* *
* Caller is responsible for notifying bgw_notify_pid, if appropriate. * Caller is responsible for notifying bgw_notify_pid, if appropriate.
* *
* This function must be invoked only in the postmaster. * This function must be invoked only in the postmaster.
*/ */
void void
ForgetBackgroundWorker(slist_mutable_iter *cur) ForgetBackgroundWorker(RegisteredBgWorker *rw)
{ {
RegisteredBgWorker *rw;
BackgroundWorkerSlot *slot; BackgroundWorkerSlot *slot;
rw = slist_container(RegisteredBgWorker, rw_lnode, cur->cur);
Assert(rw->rw_shmem_slot < max_worker_processes); Assert(rw->rw_shmem_slot < max_worker_processes);
slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot];
Assert(slot->in_use); Assert(slot->in_use);
@ -454,7 +450,7 @@ ForgetBackgroundWorker(slist_mutable_iter *cur)
(errmsg_internal("unregistering background worker \"%s\"", (errmsg_internal("unregistering background worker \"%s\"",
rw->rw_worker.bgw_name))); rw->rw_worker.bgw_name)));
slist_delete_current(cur); dlist_delete(&rw->rw_lnode);
pfree(rw); pfree(rw);
} }
@ -480,17 +476,17 @@ ReportBackgroundWorkerPID(RegisteredBgWorker *rw)
* Report that the PID of a background worker is now zero because a * Report that the PID of a background worker is now zero because a
* previously-running background worker has exited. * previously-running background worker has exited.
* *
* NOTE: The entry may be unlinked from BackgroundWorkerList. If the caller
* is iterating through it, better use a mutable iterator!
*
* This function should only be called from the postmaster. * This function should only be called from the postmaster.
*/ */
void void
ReportBackgroundWorkerExit(slist_mutable_iter *cur) ReportBackgroundWorkerExit(RegisteredBgWorker *rw)
{ {
RegisteredBgWorker *rw;
BackgroundWorkerSlot *slot; BackgroundWorkerSlot *slot;
int notify_pid; int notify_pid;
rw = slist_container(RegisteredBgWorker, rw_lnode, cur->cur);
Assert(rw->rw_shmem_slot < max_worker_processes); Assert(rw->rw_shmem_slot < max_worker_processes);
slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot];
slot->pid = rw->rw_pid; slot->pid = rw->rw_pid;
@ -505,7 +501,7 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur)
*/ */
if (rw->rw_terminate || if (rw->rw_terminate ||
rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART) rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART)
ForgetBackgroundWorker(cur); ForgetBackgroundWorker(rw);
if (notify_pid != 0) if (notify_pid != 0)
kill(notify_pid, SIGUSR1); kill(notify_pid, SIGUSR1);
@ -519,13 +515,13 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur)
void void
BackgroundWorkerStopNotifications(pid_t pid) BackgroundWorkerStopNotifications(pid_t pid)
{ {
slist_iter siter; dlist_iter iter;
slist_foreach(siter, &BackgroundWorkerList) dlist_foreach(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_worker.bgw_notify_pid == pid) if (rw->rw_worker.bgw_notify_pid == pid)
rw->rw_worker.bgw_notify_pid = 0; rw->rw_worker.bgw_notify_pid = 0;
} }
@ -546,14 +542,14 @@ BackgroundWorkerStopNotifications(pid_t pid)
void void
ForgetUnstartedBackgroundWorkers(void) ForgetUnstartedBackgroundWorkers(void)
{ {
slist_mutable_iter iter; dlist_mutable_iter iter;
slist_foreach_modify(iter, &BackgroundWorkerList) dlist_foreach_modify(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
BackgroundWorkerSlot *slot; BackgroundWorkerSlot *slot;
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
Assert(rw->rw_shmem_slot < max_worker_processes); Assert(rw->rw_shmem_slot < max_worker_processes);
slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot];
@ -564,7 +560,7 @@ ForgetUnstartedBackgroundWorkers(void)
/* ... then zap it, and notify the waiter */ /* ... then zap it, and notify the waiter */
int notify_pid = rw->rw_worker.bgw_notify_pid; int notify_pid = rw->rw_worker.bgw_notify_pid;
ForgetBackgroundWorker(&iter); ForgetBackgroundWorker(rw);
if (notify_pid != 0) if (notify_pid != 0)
kill(notify_pid, SIGUSR1); kill(notify_pid, SIGUSR1);
} }
@ -584,13 +580,13 @@ ForgetUnstartedBackgroundWorkers(void)
void void
ResetBackgroundWorkerCrashTimes(void) ResetBackgroundWorkerCrashTimes(void)
{ {
slist_mutable_iter iter; dlist_mutable_iter iter;
slist_foreach_modify(iter, &BackgroundWorkerList) dlist_foreach_modify(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART) if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART)
{ {
@ -601,7 +597,7 @@ ResetBackgroundWorkerCrashTimes(void)
* parallel_terminate_count will get incremented after we've * parallel_terminate_count will get incremented after we've
* already zeroed parallel_register_count, which would be bad.) * already zeroed parallel_register_count, which would be bad.)
*/ */
ForgetBackgroundWorker(&iter); ForgetBackgroundWorker(rw);
} }
else else
{ {
@ -1036,7 +1032,7 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
rw->rw_crashed_at = 0; rw->rw_crashed_at = 0;
rw->rw_terminate = false; rw->rw_terminate = false;
slist_push_head(&BackgroundWorkerList, &rw->rw_lnode); dlist_push_head(&BackgroundWorkerList, &rw->rw_lnode);
} }
/* /*

View File

@ -1531,7 +1531,7 @@ DetermineSleepTime(void)
if (HaveCrashedWorker) if (HaveCrashedWorker)
{ {
slist_mutable_iter siter; dlist_mutable_iter iter;
/* /*
* When there are crashed bgworkers, we sleep just long enough that * When there are crashed bgworkers, we sleep just long enough that
@ -1539,12 +1539,12 @@ DetermineSleepTime(void)
* determine the minimum of all wakeup times according to most recent * determine the minimum of all wakeup times according to most recent
* crash time and requested restart interval. * crash time and requested restart interval.
*/ */
slist_foreach_modify(siter, &BackgroundWorkerList) dlist_foreach_modify(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
TimestampTz this_wakeup; TimestampTz this_wakeup;
rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_crashed_at == 0) if (rw->rw_crashed_at == 0)
continue; continue;
@ -1552,7 +1552,7 @@ DetermineSleepTime(void)
if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART
|| rw->rw_terminate) || rw->rw_terminate)
{ {
ForgetBackgroundWorker(&siter); ForgetBackgroundWorker(rw);
continue; continue;
} }
@ -2625,13 +2625,13 @@ CleanupBackgroundWorker(int pid,
int exitstatus) /* child's exit status */ int exitstatus) /* child's exit status */
{ {
char namebuf[MAXPGPATH]; char namebuf[MAXPGPATH];
slist_mutable_iter iter; dlist_mutable_iter iter;
slist_foreach_modify(iter, &BackgroundWorkerList) dlist_foreach_modify(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_pid != pid) if (rw->rw_pid != pid)
continue; continue;
@ -2694,7 +2694,7 @@ CleanupBackgroundWorker(int pid,
rw->rw_backend = NULL; rw->rw_backend = NULL;
rw->rw_pid = 0; rw->rw_pid = 0;
rw->rw_child_slot = 0; rw->rw_child_slot = 0;
ReportBackgroundWorkerExit(&iter); /* report child death */ ReportBackgroundWorkerExit(rw); /* report child death */
LogChildExit(EXIT_STATUS_0(exitstatus) ? DEBUG1 : LOG, LogChildExit(EXIT_STATUS_0(exitstatus) ? DEBUG1 : LOG,
namebuf, pid, exitstatus); namebuf, pid, exitstatus);
@ -2796,8 +2796,8 @@ CleanupBackend(int pid,
static void static void
HandleChildCrash(int pid, int exitstatus, const char *procname) HandleChildCrash(int pid, int exitstatus, const char *procname)
{ {
dlist_mutable_iter iter; dlist_iter iter;
slist_iter siter; dlist_mutable_iter miter;
Backend *bp; Backend *bp;
bool take_action; bool take_action;
@ -2819,11 +2819,11 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
} }
/* Process background workers. */ /* Process background workers. */
slist_foreach(siter, &BackgroundWorkerList) dlist_foreach(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
if (rw->rw_pid == 0) if (rw->rw_pid == 0)
continue; /* not running */ continue; /* not running */
if (rw->rw_pid == pid) if (rw->rw_pid == pid)
@ -2853,9 +2853,9 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
} }
/* Process regular backends */ /* Process regular backends */
dlist_foreach_modify(iter, &BackendList) dlist_foreach_modify(miter, &BackendList)
{ {
bp = dlist_container(Backend, elem, iter.cur); bp = dlist_container(Backend, elem, miter.cur);
if (bp->pid == pid) if (bp->pid == pid)
{ {
@ -2866,7 +2866,7 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
{ {
(void) ReleasePostmasterChildSlot(bp->child_slot); (void) ReleasePostmasterChildSlot(bp->child_slot);
} }
dlist_delete(iter.cur); dlist_delete(miter.cur);
pfree(bp); pfree(bp);
/* Keep looping so we can signal remaining backends */ /* Keep looping so we can signal remaining backends */
} }
@ -4177,7 +4177,7 @@ maybe_start_bgworkers(void)
#define MAX_BGWORKERS_TO_LAUNCH 100 #define MAX_BGWORKERS_TO_LAUNCH 100
int num_launched = 0; int num_launched = 0;
TimestampTz now = 0; TimestampTz now = 0;
slist_mutable_iter iter; dlist_mutable_iter iter;
/* /*
* During crash recovery, we have no need to be called until the state * During crash recovery, we have no need to be called until the state
@ -4194,11 +4194,11 @@ maybe_start_bgworkers(void)
StartWorkerNeeded = false; StartWorkerNeeded = false;
HaveCrashedWorker = false; HaveCrashedWorker = false;
slist_foreach_modify(iter, &BackgroundWorkerList) dlist_foreach_modify(iter, &BackgroundWorkerList)
{ {
RegisteredBgWorker *rw; RegisteredBgWorker *rw;
rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur);
/* ignore if already running */ /* ignore if already running */
if (rw->rw_pid != 0) if (rw->rw_pid != 0)
@ -4207,7 +4207,7 @@ maybe_start_bgworkers(void)
/* if marked for death, clean up and remove from list */ /* if marked for death, clean up and remove from list */
if (rw->rw_terminate) if (rw->rw_terminate)
{ {
ForgetBackgroundWorker(&iter); ForgetBackgroundWorker(rw);
continue; continue;
} }
@ -4226,7 +4226,7 @@ maybe_start_bgworkers(void)
notify_pid = rw->rw_worker.bgw_notify_pid; notify_pid = rw->rw_worker.bgw_notify_pid;
ForgetBackgroundWorker(&iter); ForgetBackgroundWorker(rw);
/* Report worker is gone now. */ /* Report worker is gone now. */
if (notify_pid != 0) if (notify_pid != 0)

View File

@ -39,17 +39,17 @@ typedef struct RegisteredBgWorker
TimestampTz rw_crashed_at; /* if not 0, time it last crashed */ TimestampTz rw_crashed_at; /* if not 0, time it last crashed */
int rw_shmem_slot; int rw_shmem_slot;
bool rw_terminate; bool rw_terminate;
slist_node rw_lnode; /* list link */ dlist_node rw_lnode; /* list link */
} RegisteredBgWorker; } RegisteredBgWorker;
extern PGDLLIMPORT slist_head BackgroundWorkerList; extern PGDLLIMPORT dlist_head BackgroundWorkerList;
extern Size BackgroundWorkerShmemSize(void); extern Size BackgroundWorkerShmemSize(void);
extern void BackgroundWorkerShmemInit(void); extern void BackgroundWorkerShmemInit(void);
extern void BackgroundWorkerStateChange(bool allow_new_workers); extern void BackgroundWorkerStateChange(bool allow_new_workers);
extern void ForgetBackgroundWorker(slist_mutable_iter *cur); extern void ForgetBackgroundWorker(RegisteredBgWorker *rw);
extern void ReportBackgroundWorkerPID(RegisteredBgWorker *); extern void ReportBackgroundWorkerPID(RegisteredBgWorker *rw);
extern void ReportBackgroundWorkerExit(slist_mutable_iter *cur); extern void ReportBackgroundWorkerExit(RegisteredBgWorker *rw);
extern void BackgroundWorkerStopNotifications(pid_t pid); extern void BackgroundWorkerStopNotifications(pid_t pid);
extern void ForgetUnstartedBackgroundWorkers(void); extern void ForgetUnstartedBackgroundWorkers(void);
extern void ResetBackgroundWorkerCrashTimes(void); extern void ResetBackgroundWorkerCrashTimes(void);