From 5d7f521ea233fae2ac1e0956ada8d65db508c0ee Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 7 Jan 2025 21:06:51 -0500 Subject: [PATCH v1 4/4] WIP: Change shutdown sequence to terminate checkpointer last The main motivation for this change is to have a process that can serialize stats after all other processes have terminated. That already happens in checkpointer, even though walsenders can be active longer. The only reason the current state does not actively cause problems is that walsender currently generate any stats. However, there is a patch to change that. Another need for this originates in the AIO patchset, where IO workers can emit stats in some edge cases and need to run while the shutdown checkpoint is being written. This commit changes the shutdown sequence so checkpointer is signalled to trigger writing the shutdown checkpoint without terminating it. Once checkpointer wrote the checkpoint it will wait for a termination signal. Postmaster now triggers the shutdown checkpoint where we previously did so by terminating checkpointer. Checkpointer is terminated after all other children have been terminated, tracked using the new PM_SHUTDOWN_CHECKPOINTER PMState. In addition, the existing PM_SHUTDOWN_2 state is renamed to PM_XLOG_IS_SHUTDOWN, that seems a lot more descriptive. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/include/storage/pmsignal.h | 3 +- src/backend/postmaster/checkpointer.c | 103 +++++++++---- src/backend/postmaster/postmaster.c | 139 ++++++++++++------ .../utils/activity/wait_event_names.txt | 1 + 4 files changed, 170 insertions(+), 76 deletions(-) diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h index 3fbe5bf1136..d84a383047e 100644 --- a/src/include/storage/pmsignal.h +++ b/src/include/storage/pmsignal.h @@ -40,9 +40,10 @@ typedef enum PMSIGNAL_BACKGROUND_WORKER_CHANGE, /* background worker state change */ PMSIGNAL_START_WALRECEIVER, /* start a walreceiver */ PMSIGNAL_ADVANCE_STATE_MACHINE, /* advance postmaster's state machine */ + PMSIGNAL_XLOG_IS_SHUTDOWN, /* ShutdownXLOG() completed */ } PMSignalReason; -#define NUM_PMSIGNALS (PMSIGNAL_ADVANCE_STATE_MACHINE+1) +#define NUM_PMSIGNALS (PMSIGNAL_XLOG_IS_SHUTDOWN+1) /* * Reasons why the postmaster would send SIGQUIT to its children. diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 9bfd0fd665c..44e25994ca7 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -51,6 +51,7 @@ #include "storage/fd.h" #include "storage/ipc.h" #include "storage/lwlock.h" +#include "storage/pmsignal.h" #include "storage/proc.h" #include "storage/procsignal.h" #include "storage/shmem.h" @@ -141,6 +142,7 @@ double CheckPointCompletionTarget = 0.9; * Private state */ static bool ckpt_active = false; +static volatile sig_atomic_t ShutdownXLOGPending = false; /* these values are valid when ckpt_active is true: */ static pg_time_t ckpt_start_time; @@ -161,6 +163,7 @@ static void UpdateSharedMemoryConfig(void); /* Signal handlers */ static void ReqCheckpointHandler(SIGNAL_ARGS); +static void ReqShutdownXLOG(SIGNAL_ARGS); /* @@ -192,12 +195,12 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) */ pqsignal(SIGHUP, SignalHandlerForConfigReload); pqsignal(SIGINT, ReqCheckpointHandler); /* request checkpoint */ - pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */ + pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); pqsignal(SIGUSR1, procsignal_sigusr1_handler); - pqsignal(SIGUSR2, SignalHandlerForShutdownRequest); + pqsignal(SIGUSR2, ReqShutdownXLOG); /* * Reset some signals that are accepted by postmaster but not here @@ -214,8 +217,11 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) * process during a normal shutdown, and since checkpointer is shut down * very late... * - * Walsenders are shut down after the checkpointer, but currently don't - * report stats. If that changes, we need a more complicated solution. + * While e.g. walsenders are active after the shutdown checkpoint has been + * written (and thus could produce more stats), checkpointer stays around + * after the shutdown checkpoint has been written. postmaster.c will only + * signal checkpointer to exit after all processes that could emit stats + * have been shut down. */ before_shmem_exit(pgstat_before_server_shutdown, 0); @@ -330,7 +336,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) ProcGlobal->checkpointerProc = MyProcNumber; /* - * Loop forever + * Loop until we've been asked to write shutdown checkpoint. */ for (;;) { @@ -349,7 +355,10 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) * Process any requests or signals received recently. */ AbsorbSyncRequests(); + HandleCheckpointerInterrupts(); + if (ShutdownXLOGPending || ShutdownRequestPending) + break; /* * Detect a pending checkpoint request by checking whether the flags @@ -522,6 +531,8 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) /* We may have received an interrupt during the checkpoint. */ HandleCheckpointerInterrupts(); + if (ShutdownXLOGPending || ShutdownRequestPending) + break; } /* Check for archive_timeout and switch xlog files if necessary. */ @@ -560,6 +571,56 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) cur_timeout * 1000L /* convert to ms */ , WAIT_EVENT_CHECKPOINTER_MAIN); } + + /* + * From here on, elog(ERROR) should end with exit(1), not send control + * back to the sigsetjmp block above. + */ + ExitOnAnyError = true; + + if (ShutdownXLOGPending) + { + /* + * Close down the database. + * + * Since ShutdownXLOG() creates restartpoint or checkpoint, and + * updates the statistics, increment the checkpoint request and flush + * out pending statistic. + */ + PendingCheckpointerStats.num_requested++; + ShutdownXLOG(0, 0); + pgstat_report_checkpointer(); + pgstat_report_wal(true); + + /* + * Tell postmaster that we're done. + */ + SendPostmasterSignal(PMSIGNAL_XLOG_IS_SHUTDOWN); + } + + /* + * Wait until we're asked to shut down. By seperating the writing of the + * shutdown checkpoint from checkpointer exiting, checkpointer can perform + * some should-be-as-late-as-possible work like writing out stats. + */ + for (;;) + { + /* Clear any already-pending wakeups */ + ResetLatch(MyLatch); + + HandleCheckpointerInterrupts(); + + if (ShutdownRequestPending) + break; + + (void) WaitLatch(MyLatch, + WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, + 0, + WAIT_EVENT_CHECKPOINTER_SHUTDOWN); + } + + /* Normal exit from the checkpointer is here */ + proc_exit(0); /* done */ } /* @@ -589,29 +650,6 @@ HandleCheckpointerInterrupts(void) */ UpdateSharedMemoryConfig(); } - if (ShutdownRequestPending) - { - /* - * From here on, elog(ERROR) should end with exit(1), not send control - * back to the sigsetjmp block above - */ - ExitOnAnyError = true; - - /* - * Close down the database. - * - * Since ShutdownXLOG() creates restartpoint or checkpoint, and - * updates the statistics, increment the checkpoint request and flush - * out pending statistic. - */ - PendingCheckpointerStats.num_requested++; - ShutdownXLOG(0, 0); - pgstat_report_checkpointer(); - pgstat_report_wal(true); - - /* Normal exit from the checkpointer is here */ - proc_exit(0); /* done */ - } /* Perform logging of memory contexts of this process */ if (LogMemoryContextPending) @@ -732,6 +770,7 @@ CheckpointWriteDelay(int flags, double progress) * in which case we just try to catch up as quickly as possible. */ if (!(flags & CHECKPOINT_IMMEDIATE) && + !ShutdownXLOGPending && !ShutdownRequestPending && !ImmediateCheckpointRequested() && IsCheckpointOnSchedule(progress)) @@ -876,6 +915,14 @@ ReqCheckpointHandler(SIGNAL_ARGS) SetLatch(MyLatch); } +/* SIGUSR2: set flag to trigger writing of shutdown checkpoint */ +static void +ReqShutdownXLOG(SIGNAL_ARGS) +{ + ShutdownXLOGPending = true; + SetLatch(MyLatch); +} + /* -------------------------------- * communication with backends diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index d65b00f9338..2fe8e2b23b1 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -324,9 +324,10 @@ typedef enum PM_WAIT_BACKENDS, /* waiting for live backends to exit */ PM_SHUTDOWN, /* waiting for checkpointer to do shutdown * ckpt */ - PM_SHUTDOWN_2, /* waiting for archiver and walsenders to + PM_XLOG_IS_SHUTDOWN, /* waiting for archiver and walsenders to * finish */ PM_WAIT_DEAD_END, /* waiting for dead-end children to exit */ + PM_SHUTDOWN_CHECKPOINTER, /* waiting for checkpointer to shut down */ PM_NO_CHILDREN, /* all important children have exited */ } PMState; @@ -2348,35 +2349,16 @@ process_pm_child_exit(void) { ReleasePostmasterChildSlot(CheckpointerPMChild); CheckpointerPMChild = NULL; - if (EXIT_STATUS_0(exitstatus) && pmState == PM_SHUTDOWN) + if (EXIT_STATUS_0(exitstatus) && pmState == PM_SHUTDOWN_CHECKPOINTER) { /* * OK, we saw normal exit of the checkpointer after it's been - * told to shut down. We expect that it wrote a shutdown - * checkpoint. (If for some reason it didn't, recovery will - * occur on next postmaster start.) + * told to shut down. We know it wrote a shutdown checkpoint, + * otherwise we'd still be in PM_SHUTDOWN. * - * At this point we should have no normal backend children - * left (else we'd not be in PM_SHUTDOWN state) but we might - * have dead-end children to wait for. - * - * If we have an archiver subprocess, tell it to do a last - * archive cycle and quit. Likewise, if we have walsender - * processes, tell them to send any remaining WAL and quit. - */ - Assert(Shutdown > NoShutdown); - - /* Waken archiver for the last time */ - if (PgArchPMChild != NULL) - signal_child(PgArchPMChild, SIGUSR2); - - /* - * Waken walsenders for the last time. No regular backends - * should be around anymore. + * At this point we have no children left. */ - SignalChildren(SIGUSR2, btmask(B_WAL_SENDER)); - - UpdatePMState(PM_SHUTDOWN_2); + UpdatePMState(PM_NO_CHILDREN); } else { @@ -2924,9 +2906,9 @@ PostmasterStateMachine(void) SignalChildren(SIGQUIT, btmask(B_DEAD_END_BACKEND)); /* - * We already SIGQUIT'd walsenders and the archiver, if any, - * when we started immediate shutdown or entered FatalError - * state. + * We already SIGQUIT'd archiver, checkpointer and walsenders, + * if any, when we started immediate shutdown or entered + * FatalError state. */ } else @@ -2969,19 +2951,19 @@ PostmasterStateMachine(void) } } - if (pmState == PM_SHUTDOWN_2) + if (pmState == PM_XLOG_IS_SHUTDOWN) { /* - * PM_SHUTDOWN_2 state ends when there's no other children than - * dead-end children left. There shouldn't be any regular backends - * left by now anyway; what we're really waiting for is walsenders and - * archiver. + * PM_XLOG_IS_SHUTDOWN state ends when there's no children other than + * checkpointer and dead-end children left. There shouldn't be any + * regular backends left by now anyway; what we're really waiting for + * is for walsenders and archiver to exit. */ - if (CountChildren(btmask_all_except(B_LOGGER, B_DEAD_END_BACKEND)) == 0) + if (CountChildren(btmask_all_except(B_LOGGER, B_DEAD_END_BACKEND, B_CHECKPOINTER)) == 0) { UpdatePMState(PM_WAIT_DEAD_END); ConfigurePostmasterWaitSet(false); - SignalChildren(SIGTERM, btmask_all_except(B_LOGGER)); + SignalChildren(SIGTERM, btmask_all_except(B_LOGGER, B_CHECKPOINTER)); } } @@ -2989,29 +2971,51 @@ PostmasterStateMachine(void) { /* * PM_WAIT_DEAD_END state ends when all other children are gone except - * for the logger. During normal shutdown, all that remains are - * dead-end backends, but in FatalError processing we jump straight - * here with more processes remaining. Note that they have already - * been sent appropriate shutdown signals, either during a normal - * state transition leading up to PM_WAIT_DEAD_END, or during - * FatalError processing. + * for the logger and checkpointer. During normal shutdown, all that + * remains are dead-end backends and checkpointer, but in FatalError + * processing we jump straight here with more processes remaining. + * Note that they have already been sent appropriate shutdown signals, + * either during a normal state transition leading up to + * PM_WAIT_DEAD_END, or during FatalError processing. * * The reason we wait is to protect against a new postmaster starting * conflicting subprocesses; this isn't an ironclad protection, but it * at least helps in the shutdown-and-immediately-restart scenario. */ - if (CountChildren(btmask_all_except(B_LOGGER)) == 0) + if (CountChildren(btmask_all_except(B_LOGGER, B_CHECKPOINTER)) == 0) { /* These other guys should be dead already */ Assert(StartupPMChild == NULL); Assert(WalReceiverPMChild == NULL); Assert(WalSummarizerPMChild == NULL); Assert(BgWriterPMChild == NULL); - Assert(CheckpointerPMChild == NULL); Assert(WalWriterPMChild == NULL); Assert(AutoVacLauncherPMChild == NULL); Assert(SlotSyncWorkerPMChild == NULL); /* syslogger is not considered here */ + + UpdatePMState(PM_SHUTDOWN_CHECKPOINTER); + + /* + * Now that everyone else is gone, tell checkpointer to shut down + * too. That allows checkpointer to perform some last bits of of + * cleanup without other processes interfering. + */ + SignalChildren(SIGTERM, btmask(B_CHECKPOINTER)); + } + } + + if (pmState == PM_SHUTDOWN_CHECKPOINTER) + { + /* + * PM_SHUTDOWN_CHECKPOINTER ends when, drumroll, checkpointer has shut + * down. Note that checkpointer already has completed the shutdown + * checkpoint, we just wait for it to do some last bits of cleanup and + * then exit. + */ + if (CountChildren(btmask_all_except(B_LOGGER)) == 0) + { + Assert(CheckpointerPMChild == NULL); UpdatePMState(PM_NO_CHILDREN); } } @@ -3119,8 +3123,9 @@ pmstate_name(PMState state) PM_CASE(PM_STOP_BACKENDS); PM_CASE(PM_WAIT_BACKENDS); PM_CASE(PM_SHUTDOWN); - PM_CASE(PM_SHUTDOWN_2); + PM_CASE(PM_XLOG_IS_SHUTDOWN); PM_CASE(PM_WAIT_DEAD_END); + PM_CASE(PM_SHUTDOWN_CHECKPOINTER); PM_CASE(PM_NO_CHILDREN); } #undef PM_CASE @@ -3559,6 +3564,8 @@ ExitPostmaster(int status) static void process_pm_pmsignal(void) { + bool request_state_update = false; + pending_pm_pmsignal = false; ereport(DEBUG2, @@ -3670,9 +3677,46 @@ process_pm_pmsignal(void) WalReceiverRequested = true; } + if (pmState == PM_SHUTDOWN && + CheckPostmasterSignal(PMSIGNAL_XLOG_IS_SHUTDOWN)) + { + /* + * Checkpointer completed the shutdown checkpoint. + * + * If we have an archiver subprocess, tell it to do a last archive + * cycle and quit. Likewise, if we have walsender processes, tell them + * to send any remaining WAL and quit. + */ + Assert(Shutdown > NoShutdown); + + /* Waken archiver for the last time */ + if (PgArchPMChild != NULL) + signal_child(PgArchPMChild, SIGUSR2); + + /* + * Waken walsenders for the last time. No regular backends should be + * around anymore. + */ + SignalChildren(SIGUSR2, btmask(B_WAL_SENDER)); + + UpdatePMState(PM_XLOG_IS_SHUTDOWN); + + /* + * Need to run PostmasterStateMachine() to check if we already can go + * to the next state. + */ + request_state_update = true; + } + /* * Try to advance postmaster's state machine, if a child requests it. - * + */ + if (CheckPostmasterSignal(PMSIGNAL_ADVANCE_STATE_MACHINE)) + { + request_state_update = true; + } + + /* * Be careful about the order of this action relative to this function's * other actions. Generally, this should be after other actions, in case * they have effects PostmasterStateMachine would need to know about. @@ -3680,7 +3724,7 @@ process_pm_pmsignal(void) * cannot have any (immediate) effect on the state machine, but does * depend on what state we're in now. */ - if (CheckPostmasterSignal(PMSIGNAL_ADVANCE_STATE_MACHINE)) + if (request_state_update) { PostmasterStateMachine(); } @@ -3991,8 +4035,9 @@ bgworker_should_start_now(BgWorkerStartTime start_time) switch (pmState) { case PM_NO_CHILDREN: + case PM_SHUTDOWN_CHECKPOINTER: case PM_WAIT_DEAD_END: - case PM_SHUTDOWN_2: + case PM_XLOG_IS_SHUTDOWN: case PM_SHUTDOWN: case PM_WAIT_BACKENDS: case PM_STOP_BACKENDS: diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index 0b53cba807d..e199f071628 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -56,6 +56,7 @@ AUTOVACUUM_MAIN "Waiting in main loop of autovacuum launcher process." BGWRITER_HIBERNATE "Waiting in background writer process, hibernating." BGWRITER_MAIN "Waiting in main loop of background writer process." CHECKPOINTER_MAIN "Waiting in main loop of checkpointer process." +CHECKPOINTER_SHUTDOWN "Waiting for checkpointer process to be terminated." LOGICAL_APPLY_MAIN "Waiting in main loop of logical replication apply process." LOGICAL_LAUNCHER_MAIN "Waiting in main loop of logical replication launcher process." LOGICAL_PARALLEL_APPLY_MAIN "Waiting in main loop of logical replication parallel apply process." -- 2.45.2.746.g06e570c0df.dirty