Hi, I've pushed 0001-0005.
I think 0006 isn't far from ready. However, I think 0007 needs a bit more work. One thing that worries me a bit is that using SIGINT for triggering the shutdown checkpoint could somehow be unintentionally triggered? Previously checkpointer would effectively have ignored that, but now it'd bring the whole cluster down (because postmaster would see an unexpected ShutdownXLOG()). But I can't really see a legitimate reason for checkpointer to get spurious SIGINTs. This made me think about what happens if a spurious SIGINT is sent - and in the last version the answer wasn't great, namely everything seemed to go on normal, except that the cluster couldn't be shut down normally. There wasn't any code to treat receiving PMSIGNAL_XLOG_IS_SHUTDOWN in the wrong state as fatal. I think this needs to be treated the same way we treat not being able to fork checkpointer during a shutdown. Now receiving PMSIGNAL_XLOG_IS_SHUTDOWN while not in PM_WAIT_XLOG_SHUTDOWN triggers FatalError processing after logging "WAL was shut down unexpectedly". We have multiple copies of code to go to FatalError, that doesn't seem great. Another thing I started worrying about is that the commit added PM_WAIT_CHECKPOINTER *after* PM_WAIT_DEAD_END. However, we have a few places where we directly jump to PM_WAIT_DEAD_END in fatal situations: /* * Stop any dead-end children and stop creating new ones. */ UpdatePMState(PM_WAIT_DEAD_END); ConfigurePostmasterWaitSet(false); SignalChildren(SIGQUIT, btmask(B_DEAD_END_BACKEND)); and /* * If we failed to fork a checkpointer, just shut down. * Any required cleanup will happen at next restart. We * set FatalError so that an "abnormal shutdown" message * gets logged when we exit. * * We don't consult send_abort_for_crash here, as it's * unlikely that dumping cores would illuminate the reason * for checkpointer fork failure. */ FatalError = true; UpdatePMState(PM_WAIT_DEAD_END); ConfigurePostmasterWaitSet(false); I don't think this actively causes problems today, but it seems rather iffy. Maybe PM_WAIT_CHECKPOINTER should be before PM_WAIT_DEAD_END? I earlier thought it'd be better to shut checkpointer down after even dead-end children exited, in case we ever wanted to introduce stats for dead-end children - but that seems rather unlikely? I also noticed that checkpointer.c intro comment needed some editing. Attached are the rebased and edited last two patches. Independent of this patch, we seem to deal with FatalError processing in a somewhat inconsistently. We have this comment (in master): /* * 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. However that doesn't actually appear to be true in the most common way to reach FatalError, via HandleChildCrash(): if (Shutdown != ImmediateShutdown) FatalError = true; /* We now transit into a state of waiting for children to die */ if (pmState == PM_RECOVERY || pmState == PM_HOT_STANDBY || pmState == PM_RUN || pmState == PM_STOP_BACKENDS || pmState == PM_WAIT_XLOG_SHUTDOWN) UpdatePMState(PM_WAIT_BACKENDS); Here we a) don't directly go to PM_WAIT_DEAD_END, but PM_WAIT_BACKENDS From PM_WAIT_BACKENDS we'll switch to PM_WAIT_DEAD_END if all processes other than walsender, archiver and dead-end children exited. b) in PM_WAIT_XLOG_SHUTDOWN (previously named PM_SHUTDOWN) we go *back* to PM_WAIT_BACKENDS? This comment seems to have been added in commit a78af0427015449269fb7d9d8c6057cfcb740149 Author: Heikki Linnakangas <heikki.linnakan...@iki.fi> Date: 2024-11-14 16:12:28 +0200 Assign a child slot to every postmaster child process ISTM that section of the comment is largely wrong and has never really been the case if my git sleuthing is correct? We do have one place that directly switches to PM_WAIT_DEAD_END: /* * If we failed to fork a checkpointer, just shut down. * Any required cleanup will happen at next restart. We * set FatalError so that an "abnormal shutdown" message * gets logged when we exit. * * We don't consult send_abort_for_crash here, as it's * unlikely that dumping cores would illuminate the reason * for checkpointer fork failure. */ FatalError = true; UpdatePMState(PM_WAIT_DEAD_END); ConfigurePostmasterWaitSet(false); but I suspect that's just about unreachable these days. If checkpointer exits outside of shutdown processing, it's treated as as a reason to crash-restart: /* * Any unexpected exit of the checkpointer (including FATAL * exit) is treated as a crash. */ HandleChildCrash(pid, exitstatus, _("checkpointer process")); During postmaster's first start checkpointer is launched before the startup process is even started. During crash restart processing, we do start the startup process before checkpointer, but start checkpointer in LaunchMissingBackgroundProcesses(). It looks like it's theoretically possible that recovery completes before ever reach LaunchMissingBackgroundProcesses() - but it seems like we should address that by having the same "process startup order" during crash recovery processing as we have during the first start? I think this oddity originated in commit 7ff23c6d277d1d90478a51f0dd81414d343f3850 Author: Thomas Munro <tmu...@postgresql.org> Date: 2021-08-02 17:32:20 +1200 Run checkpointer and bgwriter in crash recovery. ISTM that we should change it so that the first and crash recovery scenarios are more similar. Thomas, what do you think? Separately, is it a problem that somewhere in process_pm_* we did ConfigurePostmasterWaitSet(false) but then the event loop in ServerLoop() continues to process "old" WL_SOCKET_ACCEPT events? Greetings, Andres Freund
>From 16289910f50ed92aee08d1320c31235e83273999 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Fri, 10 Jan 2025 11:11:40 -0500 Subject: [PATCH v3 1/2] checkpointer: Request checkpoint via latch instead of signal The main reason for this is that a future commit would like to use SIGINT for another purpose. But it's also a tad nicer and tad more efficient to use SetLatch(), as it avoids a signal when checkpointer already is busy. Reviewed-by: Bertrand Drouvot <bertranddrouvot...@gmail.com> Reviewed-by: Nazir Bilal Yavuz <byavu...@gmail.com> Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp --- src/backend/postmaster/checkpointer.c | 60 +++++++++------------------ 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 9bfd0fd665c..dd2c8376c6e 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -159,9 +159,6 @@ static bool ImmediateCheckpointRequested(void); static bool CompactCheckpointerRequestQueue(void); static void UpdateSharedMemoryConfig(void); -/* Signal handlers */ -static void ReqCheckpointHandler(SIGNAL_ARGS); - /* * Main entry point for checkpointer process @@ -191,7 +188,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) * tell us it's okay to shut down (via SIGUSR2). */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, ReqCheckpointHandler); /* request checkpoint */ + pqsignal(SIGINT, SIG_IGN); pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */ /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); @@ -860,23 +857,6 @@ IsCheckpointOnSchedule(double progress) } -/* -------------------------------- - * signal handler routines - * -------------------------------- - */ - -/* SIGINT: set flag to run a normal checkpoint right away */ -static void -ReqCheckpointHandler(SIGNAL_ARGS) -{ - /* - * The signaling process should have set ckpt_flags nonzero, so all we - * need do is ensure that our main loop gets kicked out of any wait. - */ - SetLatch(MyLatch); -} - - /* -------------------------------- * communication with backends * -------------------------------- @@ -990,38 +970,36 @@ RequestCheckpoint(int flags) SpinLockRelease(&CheckpointerShmem->ckpt_lck); /* - * Send signal to request checkpoint. It's possible that the checkpointer - * hasn't started yet, or is in process of restarting, so we will retry a - * few times if needed. (Actually, more than a few times, since on slow - * or overloaded buildfarm machines, it's been observed that the - * checkpointer can take several seconds to start.) However, if not told - * to wait for the checkpoint to occur, we consider failure to send the - * signal to be nonfatal and merely LOG it. The checkpointer should see - * the request when it does start, with or without getting a signal. + * Set checkpointer's latch to request checkpoint. It's possible that the + * checkpointer hasn't started yet, so we will retry a few times if + * needed. (Actually, more than a few times, since on slow or overloaded + * buildfarm machines, it's been observed that the checkpointer can take + * several seconds to start.) However, if not told to wait for the + * checkpoint to occur, we consider failure to set the latch to be + * nonfatal and merely LOG it. The checkpointer should see the request + * when it does start, with or without the SetLatch(). */ #define MAX_SIGNAL_TRIES 600 /* max wait 60.0 sec */ for (ntries = 0;; ntries++) { - if (CheckpointerShmem->checkpointer_pid == 0) + volatile PROC_HDR *procglobal = ProcGlobal; + ProcNumber checkpointerProc = procglobal->checkpointerProc; + + if (checkpointerProc == INVALID_PROC_NUMBER) { if (ntries >= MAX_SIGNAL_TRIES || !(flags & CHECKPOINT_WAIT)) { elog((flags & CHECKPOINT_WAIT) ? ERROR : LOG, - "could not signal for checkpoint: checkpointer is not running"); - break; - } - } - else if (kill(CheckpointerShmem->checkpointer_pid, SIGINT) != 0) - { - if (ntries >= MAX_SIGNAL_TRIES || !(flags & CHECKPOINT_WAIT)) - { - elog((flags & CHECKPOINT_WAIT) ? ERROR : LOG, - "could not signal for checkpoint: %m"); + "could not notify checkpoint: checkpointer is not running"); break; } } else - break; /* signal sent successfully */ + { + SetLatch(&GetPGProcByNumber(checkpointerProc)->procLatch); + /* notified successfully */ + break; + } CHECK_FOR_INTERRUPTS(); pg_usleep(100000L); /* wait 0.1 sec, then retry */ -- 2.45.2.746.g06e570c0df.dirty
>From 38a8cba733683e37db630240e683eef09498a078 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Fri, 10 Jan 2025 11:45:13 -0500 Subject: [PATCH v3 2/2] 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. Serializing stats 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 (which, in some edge cases, can emit stats of their own) need to run while the shutdown checkpoint is being written. This commit changes the shutdown sequence so checkpointer is signalled (via SIGINT) to trigger writing the shutdown checkpoint without terminating it. Once checkpointer wrote the checkpoint it will wait for a termination signal (SIGUSR2, as before). Postmaster now triggers the shutdown checkpoint via SIGINT, where we previously did so by terminating checkpointer. Checkpointer now is terminated after all other children have been terminated, tracked using the new PM_WAIT_CHECKPOINTER PMState. Reviewed-by: Heikki Linnakangas <hlinn...@iki.fi> Reviewed-by: Bertrand Drouvot <bertranddrouvot...@gmail.com> Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp --- src/include/storage/pmsignal.h | 3 +- src/backend/postmaster/checkpointer.c | 125 +++++++++---- src/backend/postmaster/postmaster.c | 165 +++++++++++++----- .../utils/activity/wait_event_names.txt | 1 + 4 files changed, 216 insertions(+), 78 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 dd2c8376c6e..767bf9f5cf8 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -10,10 +10,13 @@ * fill WAL segments; the checkpointer itself doesn't watch for the * condition.) * - * Normal termination is by SIGUSR2, which instructs the checkpointer to - * execute a shutdown checkpoint and then exit(0). (All backends must be - * stopped before SIGUSR2 is issued!) Emergency termination is by SIGQUIT; - * like any backend, the checkpointer will simply abort and exit on SIGQUIT. + * The normal termination sequence is that checkpointer is instructed to + * execute the shutdown checkpoint by SIGINT. After that checkpointer waits + * to be terminated via SIGUSR2, which instructs the checkpointer to exit(0). + * All backends must be stopped before SIGINT or SIGUSR2 is issued! + * + * Emergency termination is by SIGQUIT; like any backend, the checkpointer + * will simply abort and exit on SIGQUIT. * * If the checkpointer exits unexpectedly, the postmaster treats that the same * as a backend crash: shared memory may be corrupted, so remaining backends @@ -51,6 +54,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 +145,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; @@ -159,6 +164,9 @@ static bool ImmediateCheckpointRequested(void); static bool CompactCheckpointerRequestQueue(void); static void UpdateSharedMemoryConfig(void); +/* Signal handlers */ +static void ReqShutdownXLOG(SIGNAL_ARGS); + /* * Main entry point for checkpointer process @@ -188,7 +196,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) * tell us it's okay to shut down (via SIGUSR2). */ pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGINT, SIG_IGN); + pqsignal(SIGINT, ReqShutdownXLOG); pqsignal(SIGTERM, SIG_IGN); /* ignore SIGTERM */ /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); @@ -211,8 +219,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 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); @@ -327,7 +338,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 or terminate. */ for (;;) { @@ -346,7 +357,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 @@ -517,8 +531,13 @@ CheckpointerMain(char *startup_data, size_t startup_data_len) ckpt_active = false; - /* We may have received an interrupt during the checkpoint. */ + /* + * We may have received an interrupt during the checkpoint and the + * latch might have been reset (e.g. in CheckpointWriteDelay). + */ HandleCheckpointerInterrupts(); + if (ShutdownXLOGPending || ShutdownRequestPending) + break; } /* Check for archive_timeout and switch xlog files if necessary. */ @@ -557,6 +576,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 separating 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 */ } /* @@ -586,29 +655,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) @@ -729,6 +775,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)) @@ -857,6 +904,20 @@ IsCheckpointOnSchedule(double progress) } +/* -------------------------------- + * signal handler routines + * -------------------------------- + */ + +/* SIGINT: 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 5f615d0f605..831a2afbd8f 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -335,6 +335,7 @@ typedef enum PM_WAIT_XLOG_ARCHIVAL, /* waiting for archiver and walsenders to * finish */ PM_WAIT_DEAD_END, /* waiting for dead-end children to exit */ + PM_WAIT_CHECKPOINTER, /* waiting for checkpointer to shut down */ PM_NO_CHILDREN, /* all important children have exited */ } PMState; @@ -2355,35 +2356,17 @@ process_pm_child_exit(void) { ReleasePostmasterChildSlot(CheckpointerPMChild); CheckpointerPMChild = NULL; - if (EXIT_STATUS_0(exitstatus) && pmState == PM_WAIT_XLOG_SHUTDOWN) + if (EXIT_STATUS_0(exitstatus) && pmState == PM_WAIT_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 checkpointer wrote a shutdown + * checkpoint, otherwise we'd still be in + * PM_WAIT_XLOG_SHUTDOWN state. * - * At this point we should have no normal backend children - * left (else we'd not be in PM_WAIT_XLOG_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_WAIT_XLOG_ARCHIVAL); + UpdatePMState(PM_NO_CHILDREN); } else { @@ -2938,9 +2921,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 @@ -2954,10 +2937,10 @@ PostmasterStateMachine(void) /* Start the checkpointer if not running */ if (CheckpointerPMChild == NULL) CheckpointerPMChild = StartChildProcess(B_CHECKPOINTER); - /* And tell it to shut down */ + /* And tell it to write the shutdown checkpoint */ if (CheckpointerPMChild != NULL) { - signal_child(CheckpointerPMChild, SIGUSR2); + signal_child(CheckpointerPMChild, SIGINT); UpdatePMState(PM_WAIT_XLOG_SHUTDOWN); } else @@ -2986,16 +2969,16 @@ PostmasterStateMachine(void) if (pmState == PM_WAIT_XLOG_ARCHIVAL) { /* - * PM_WAIT_XLOG_ARCHIVAL 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_WAIT_XLOG_ARCHIVAL 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)); } } @@ -3003,29 +2986,52 @@ 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_WAIT_CHECKPOINTER); + + /* + * Now that everyone else is gone, tell checkpointer to shut down + * too. That allows checkpointer to perform some last bits of + * cleanup without other processes interfering. + */ + if (CheckpointerPMChild != NULL) + signal_child(CheckpointerPMChild, SIGUSR2); + } + } + + if (pmState == PM_WAIT_CHECKPOINTER) + { + /* + * PM_WAIT_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); } } @@ -3135,6 +3141,7 @@ pmstate_name(PMState state) PM_TOSTR_CASE(PM_WAIT_XLOG_SHUTDOWN); PM_TOSTR_CASE(PM_WAIT_XLOG_ARCHIVAL); PM_TOSTR_CASE(PM_WAIT_DEAD_END); + PM_TOSTR_CASE(PM_WAIT_CHECKPOINTER); PM_TOSTR_CASE(PM_NO_CHILDREN); } #undef PM_TOSTR_CASE @@ -3565,6 +3572,8 @@ ExitPostmaster(int status) static void process_pm_pmsignal(void) { + bool request_state_update = false; + pending_pm_pmsignal = false; ereport(DEBUG2, @@ -3676,9 +3685,74 @@ process_pm_pmsignal(void) WalReceiverRequested = true; } + if (CheckPostmasterSignal(PMSIGNAL_XLOG_IS_SHUTDOWN)) + { + /* Checkpointer completed the shutdown checkpoint */ + if (pmState == PM_WAIT_XLOG_SHUTDOWN) + { + /* + * 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_WAIT_XLOG_ARCHIVAL); + } + else if (!FatalError && Shutdown < ImmediateShutdown) + { + /* + * Checkpointer only ought to perform the shutdown checkpoint + * during shutdown. If somehow checkpointer did so in another + * situation, we have no choice but to crash-restart. + * + * It's possible however that we get PMSIGNAL_XLOG_IS_SHUTDOWN + * outside of PM_WAIT_XLOG_SHUTDOWN if an orderly shutdown was + * "interrupted" by a crash or an immediate shutdown. + * + * FIXME: This should probably not have a copy fo the code to + * transition into FatalError state. + */ + ereport(LOG, + (errmsg("WAL was shut down unexpectedly"))); + + FatalError = true; + UpdatePMState(PM_WAIT_DEAD_END); + ConfigurePostmasterWaitSet(false); + + /* + * Doesn't seem likely to help to take send_abort_for_crash into + * account here. + */ + TerminateChildren(SIGQUIT); + } + + /* + * 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. @@ -3686,7 +3760,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(); } @@ -3997,6 +4071,7 @@ bgworker_should_start_now(BgWorkerStartTime start_time) switch (pmState) { case PM_NO_CHILDREN: + case PM_WAIT_CHECKPOINTER: case PM_WAIT_DEAD_END: case PM_WAIT_XLOG_ARCHIVAL: case PM_WAIT_XLOG_SHUTDOWN: 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