From c5881b9f4b89b762cd8ef925936eec3602b565b5 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 29 Jul 2024 23:14:00 +0300 Subject: [PATCH v3 01/12] 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. Discussion: https://www.postgresql.org/message-id/835232c0-a5f7-4f20-b95b-5b56ba57d741@iki.fi --- src/backend/postmaster/bgworker.c | 62 ++++++++++----------- src/backend/postmaster/postmaster.c | 40 ++++++------- src/include/postmaster/bgworker_internals.h | 10 ++-- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 77707bb384..981d8177b0 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -37,7 +37,7 @@ /* * 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 @@ -168,7 +168,7 @@ BackgroundWorkerShmemInit(void) &found); if (!IsUnderPostmaster) { - slist_iter siter; + dlist_iter iter; int slotno = 0; BackgroundWorkerData->total_slots = max_worker_processes; @@ -181,12 +181,12 @@ BackgroundWorkerShmemInit(void) * correspondence between the postmaster's private list and the array * in shared memory. */ - slist_foreach(siter, &BackgroundWorkerList) + dlist_foreach(iter, &BackgroundWorkerList) { BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno]; RegisteredBgWorker *rw; - rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); + rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur); Assert(slotno < max_worker_processes); slot->in_use = true; slot->terminate = false; @@ -220,13 +220,13 @@ BackgroundWorkerShmemInit(void) static RegisteredBgWorker * FindRegisteredWorkerBySlotNumber(int slotno) { - slist_iter siter; + dlist_iter iter; - slist_foreach(siter, &BackgroundWorkerList) + dlist_foreach(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; - rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); + rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur); if (rw->rw_shmem_slot == slotno) return rw; } @@ -413,29 +413,25 @@ BackgroundWorkerStateChange(bool allow_new_workers) (errmsg_internal("registering background worker \"%s\"", 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. * - * The worker must be identified by passing an slist_mutable_iter that - * points to it. This convention allows deletion of workers during - * searches of the worker list, and saves having to search the list again. + * NOTE: The entry is unlinked from BackgroundWorkerList. If the caller is + * iterating through it, better use a mutable iterator! * * Caller is responsible for notifying bgw_notify_pid, if appropriate. * * This function must be invoked only in the postmaster. */ void -ForgetBackgroundWorker(slist_mutable_iter *cur) +ForgetBackgroundWorker(RegisteredBgWorker *rw) { - RegisteredBgWorker *rw; BackgroundWorkerSlot *slot; - rw = slist_container(RegisteredBgWorker, rw_lnode, cur->cur); - Assert(rw->rw_shmem_slot < max_worker_processes); slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; Assert(slot->in_use); @@ -454,7 +450,7 @@ ForgetBackgroundWorker(slist_mutable_iter *cur) (errmsg_internal("unregistering background worker \"%s\"", rw->rw_worker.bgw_name))); - slist_delete_current(cur); + dlist_delete(&rw->rw_lnode); pfree(rw); } @@ -480,17 +476,17 @@ ReportBackgroundWorkerPID(RegisteredBgWorker *rw) * Report that the PID of a background worker is now zero because a * 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. */ void -ReportBackgroundWorkerExit(slist_mutable_iter *cur) +ReportBackgroundWorkerExit(RegisteredBgWorker *rw) { - RegisteredBgWorker *rw; BackgroundWorkerSlot *slot; int notify_pid; - rw = slist_container(RegisteredBgWorker, rw_lnode, cur->cur); - Assert(rw->rw_shmem_slot < max_worker_processes); slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; slot->pid = rw->rw_pid; @@ -505,7 +501,7 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur) */ if (rw->rw_terminate || rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART) - ForgetBackgroundWorker(cur); + ForgetBackgroundWorker(rw); if (notify_pid != 0) kill(notify_pid, SIGUSR1); @@ -519,13 +515,13 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur) void BackgroundWorkerStopNotifications(pid_t pid) { - slist_iter siter; + dlist_iter iter; - slist_foreach(siter, &BackgroundWorkerList) + dlist_foreach(iter, &BackgroundWorkerList) { 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) rw->rw_worker.bgw_notify_pid = 0; } @@ -546,14 +542,14 @@ BackgroundWorkerStopNotifications(pid_t pid) void ForgetUnstartedBackgroundWorkers(void) { - slist_mutable_iter iter; + dlist_mutable_iter iter; - slist_foreach_modify(iter, &BackgroundWorkerList) + dlist_foreach_modify(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; 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); slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; @@ -564,7 +560,7 @@ ForgetUnstartedBackgroundWorkers(void) /* ... then zap it, and notify the waiter */ int notify_pid = rw->rw_worker.bgw_notify_pid; - ForgetBackgroundWorker(&iter); + ForgetBackgroundWorker(rw); if (notify_pid != 0) kill(notify_pid, SIGUSR1); } @@ -584,13 +580,13 @@ ForgetUnstartedBackgroundWorkers(void) void ResetBackgroundWorkerCrashTimes(void) { - slist_mutable_iter iter; + dlist_mutable_iter iter; - slist_foreach_modify(iter, &BackgroundWorkerList) + dlist_foreach_modify(iter, &BackgroundWorkerList) { 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) { @@ -601,7 +597,7 @@ ResetBackgroundWorkerCrashTimes(void) * parallel_terminate_count will get incremented after we've * already zeroed parallel_register_count, which would be bad.) */ - ForgetBackgroundWorker(&iter); + ForgetBackgroundWorker(rw); } else { @@ -1036,7 +1032,7 @@ RegisterBackgroundWorker(BackgroundWorker *worker) rw->rw_crashed_at = 0; rw->rw_terminate = false; - slist_push_head(&BackgroundWorkerList, &rw->rw_lnode); + dlist_push_head(&BackgroundWorkerList, &rw->rw_lnode); } /* diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index a3e9e8fdc0..fc00e39c44 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1531,7 +1531,7 @@ DetermineSleepTime(void) if (HaveCrashedWorker) { - slist_mutable_iter siter; + dlist_mutable_iter iter; /* * 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 * crash time and requested restart interval. */ - slist_foreach_modify(siter, &BackgroundWorkerList) + dlist_foreach_modify(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; 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) continue; @@ -1552,7 +1552,7 @@ DetermineSleepTime(void) if (rw->rw_worker.bgw_restart_time == BGW_NEVER_RESTART || rw->rw_terminate) { - ForgetBackgroundWorker(&siter); + ForgetBackgroundWorker(rw); continue; } @@ -2625,13 +2625,13 @@ CleanupBackgroundWorker(int pid, int exitstatus) /* child's exit status */ { char namebuf[MAXPGPATH]; - slist_mutable_iter iter; + dlist_mutable_iter iter; - slist_foreach_modify(iter, &BackgroundWorkerList) + dlist_foreach_modify(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; - rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); + rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur); if (rw->rw_pid != pid) continue; @@ -2694,7 +2694,7 @@ CleanupBackgroundWorker(int pid, rw->rw_backend = NULL; rw->rw_pid = 0; rw->rw_child_slot = 0; - ReportBackgroundWorkerExit(&iter); /* report child death */ + ReportBackgroundWorkerExit(rw); /* report child death */ LogChildExit(EXIT_STATUS_0(exitstatus) ? DEBUG1 : LOG, namebuf, pid, exitstatus); @@ -2796,8 +2796,8 @@ CleanupBackend(int pid, static void HandleChildCrash(int pid, int exitstatus, const char *procname) { - dlist_mutable_iter iter; - slist_iter siter; + dlist_iter iter; + dlist_mutable_iter miter; Backend *bp; bool take_action; @@ -2819,11 +2819,11 @@ HandleChildCrash(int pid, int exitstatus, const char *procname) } /* Process background workers. */ - slist_foreach(siter, &BackgroundWorkerList) + dlist_foreach(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; - rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur); + rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur); if (rw->rw_pid == 0) continue; /* not running */ if (rw->rw_pid == pid) @@ -2853,9 +2853,9 @@ HandleChildCrash(int pid, int exitstatus, const char *procname) } /* 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) { @@ -2866,7 +2866,7 @@ HandleChildCrash(int pid, int exitstatus, const char *procname) { (void) ReleasePostmasterChildSlot(bp->child_slot); } - dlist_delete(iter.cur); + dlist_delete(miter.cur); pfree(bp); /* Keep looping so we can signal remaining backends */ } @@ -4177,7 +4177,7 @@ maybe_start_bgworkers(void) #define MAX_BGWORKERS_TO_LAUNCH 100 int num_launched = 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 @@ -4194,11 +4194,11 @@ maybe_start_bgworkers(void) StartWorkerNeeded = false; HaveCrashedWorker = false; - slist_foreach_modify(iter, &BackgroundWorkerList) + dlist_foreach_modify(iter, &BackgroundWorkerList) { RegisteredBgWorker *rw; - rw = slist_container(RegisteredBgWorker, rw_lnode, iter.cur); + rw = dlist_container(RegisteredBgWorker, rw_lnode, iter.cur); /* ignore if already running */ if (rw->rw_pid != 0) @@ -4207,7 +4207,7 @@ maybe_start_bgworkers(void) /* if marked for death, clean up and remove from list */ if (rw->rw_terminate) { - ForgetBackgroundWorker(&iter); + ForgetBackgroundWorker(rw); continue; } @@ -4226,7 +4226,7 @@ maybe_start_bgworkers(void) notify_pid = rw->rw_worker.bgw_notify_pid; - ForgetBackgroundWorker(&iter); + ForgetBackgroundWorker(rw); /* Report worker is gone now. */ if (notify_pid != 0) diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h index 61ba54117a..e55e38af65 100644 --- a/src/include/postmaster/bgworker_internals.h +++ b/src/include/postmaster/bgworker_internals.h @@ -39,17 +39,17 @@ typedef struct RegisteredBgWorker TimestampTz rw_crashed_at; /* if not 0, time it last crashed */ int rw_shmem_slot; bool rw_terminate; - slist_node rw_lnode; /* list link */ + dlist_node rw_lnode; /* list link */ } RegisteredBgWorker; -extern PGDLLIMPORT slist_head BackgroundWorkerList; +extern PGDLLIMPORT dlist_head BackgroundWorkerList; extern Size BackgroundWorkerShmemSize(void); extern void BackgroundWorkerShmemInit(void); extern void BackgroundWorkerStateChange(bool allow_new_workers); -extern void ForgetBackgroundWorker(slist_mutable_iter *cur); -extern void ReportBackgroundWorkerPID(RegisteredBgWorker *); -extern void ReportBackgroundWorkerExit(slist_mutable_iter *cur); +extern void ForgetBackgroundWorker(RegisteredBgWorker *rw); +extern void ReportBackgroundWorkerPID(RegisteredBgWorker *rw); +extern void ReportBackgroundWorkerExit(RegisteredBgWorker *rw); extern void BackgroundWorkerStopNotifications(pid_t pid); extern void ForgetUnstartedBackgroundWorkers(void); extern void ResetBackgroundWorkerCrashTimes(void); -- 2.39.2