From 7ff23c6d277d1d90478a51f0dd81414d343f3850 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 2 Aug 2021 17:32:20 +1200 Subject: [PATCH] Run checkpointer and bgwriter in crash recovery. Start up the checkpointer and bgwriter during crash recovery (except in --single mode), as we do for replication. This wasn't done back in commit cdd46c76 out of caution. Now it seems like a better idea to make the environment as similar as possible in both cases. There may also be some performance advantages. Reviewed-by: Robert Haas Reviewed-by: Aleksander Alekseev Tested-by: Jakub Wartak Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com --- src/backend/access/transam/xlog.c | 33 ++++++++++----------------- src/backend/postmaster/bgwriter.c | 3 --- src/backend/postmaster/checkpointer.c | 3 --- src/backend/postmaster/postmaster.c | 17 ++++++-------- src/backend/storage/sync/sync.c | 30 +++--------------------- src/include/storage/sync.h | 1 - 6 files changed, 22 insertions(+), 65 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index efb3ca273ed..f84c0bb01eb 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -869,9 +869,6 @@ bool reachedConsistency = false; static bool InRedo = false; -/* Have we launched bgwriter during recovery? */ -static bool bgwriterLaunched = false; - /* For WALInsertLockAcquire/Release functions */ static int MyLockNo = 0; static bool holdingAllLocks = false; @@ -7311,25 +7308,15 @@ StartupXLOG(void) /* Also ensure XLogReceiptTime has a sane value */ XLogReceiptTime = GetCurrentTimestamp(); + /* Allow ProcSendSignal() to find us, for buffer pin wakeups. */ + PublishStartupProcessInformation(); + /* * Let postmaster know we've started redo now, so that it can launch - * checkpointer to perform restartpoints. We don't bother during - * crash recovery as restartpoints can only be performed during - * archive recovery. And we'd like to keep crash recovery simple, to - * avoid introducing bugs that could affect you when recovering after - * crash. - * - * After this point, we can no longer assume that we're the only - * process in addition to postmaster! Also, fsync requests are - * subsequently to be handled by the checkpointer, not locally. + * the archiver if necessary. */ - if (ArchiveRecoveryRequested && IsUnderPostmaster) - { - PublishStartupProcessInformation(); - EnableSyncRequestForwarding(); + if (IsUnderPostmaster) SendPostmasterSignal(PMSIGNAL_RECOVERY_STARTED); - bgwriterLaunched = true; - } /* * Allow read-only connections immediately if we're consistent @@ -7903,7 +7890,7 @@ StartupXLOG(void) * after we're fully out of recovery mode and already accepting * queries. */ - if (bgwriterLaunched) + if (ArchiveRecoveryRequested && IsUnderPostmaster) { if (LocalPromoteIsTriggered) { @@ -7927,7 +7914,11 @@ StartupXLOG(void) CHECKPOINT_WAIT); } else - CreateCheckPoint(CHECKPOINT_END_OF_RECOVERY | CHECKPOINT_IMMEDIATE); + { + RequestCheckpoint(CHECKPOINT_END_OF_RECOVERY | + CHECKPOINT_IMMEDIATE | + CHECKPOINT_WAIT); + } } if (ArchiveRecoveryRequested) @@ -12182,7 +12173,7 @@ XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen, * Request a restartpoint if we've replayed too much xlog since the * last one. */ - if (bgwriterLaunched) + if (ArchiveRecoveryRequested && IsUnderPostmaster) { if (XLogCheckpointNeeded(readSegNo)) { diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 715d5195bb6..5584f4bc241 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -12,9 +12,6 @@ * * As of Postgres 9.2 the bgwriter no longer handles checkpoints. * - * The bgwriter is started by the postmaster as soon as the startup subprocess - * finishes, or as soon as recovery begins if we are doing archive recovery. - * It remains alive until the postmaster commands it to terminate. * Normal termination is by SIGTERM, which instructs the bgwriter to exit(0). * Emergency termination is by SIGQUIT; like any backend, the bgwriter will * simply abort and exit on SIGQUIT. diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 75a95f3de7a..bc9ac7ccfaf 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -10,9 +10,6 @@ * fill WAL segments; the checkpointer itself doesn't watch for the * condition.) * - * The checkpointer is started by the postmaster as soon as the startup - * subprocess finishes, or as soon as recovery begins if we are doing archive - * recovery. It remains alive until the postmaster commands it to terminate. * 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; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 122c2b05bdb..00d051d520e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1403,6 +1403,12 @@ PostmasterMain(int argc, char *argv[]) */ AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STARTING); + /* Start bgwriter and checkpointer so they can help with recovery */ + if (CheckpointerPID == 0) + CheckpointerPID = StartCheckpointer(); + if (BgWriterPID == 0) + BgWriterPID = StartBackgroundWriter(); + /* * We're ready to rock and roll... */ @@ -1765,7 +1771,7 @@ ServerLoop(void) * fails, we'll just try again later. Likewise for the checkpointer. */ if (pmState == PM_RUN || pmState == PM_RECOVERY || - pmState == PM_HOT_STANDBY) + pmState == PM_HOT_STANDBY || pmState == PM_STARTUP) { if (CheckpointerPID == 0) CheckpointerPID = StartCheckpointer(); @@ -5161,15 +5167,6 @@ sigusr1_handler(SIGNAL_ARGS) FatalError = false; AbortStartTime = 0; - /* - * Crank up the background tasks. It doesn't matter if this fails, - * we'll just try again later. - */ - Assert(CheckpointerPID == 0); - CheckpointerPID = StartCheckpointer(); - Assert(BgWriterPID == 0); - BgWriterPID = StartBackgroundWriter(); - /* * Start the archiver if we're responsible for (re-)archiving received * files. diff --git a/src/backend/storage/sync/sync.c b/src/backend/storage/sync/sync.c index bc3ceb27125..4a2ed414b00 100644 --- a/src/backend/storage/sync/sync.c +++ b/src/backend/storage/sync/sync.c @@ -129,10 +129,10 @@ InitSync(void) { /* * Create pending-operations hashtable if we need it. Currently, we need - * it if we are standalone (not under a postmaster) or if we are a startup - * or checkpointer auxiliary process. + * it if we are standalone (not under a postmaster) or if we are a + * checkpointer auxiliary process. */ - if (!IsUnderPostmaster || AmStartupProcess() || AmCheckpointerProcess()) + if (!IsUnderPostmaster || AmCheckpointerProcess()) { HASHCTL hash_ctl; @@ -589,27 +589,3 @@ RegisterSyncRequest(const FileTag *ftag, SyncRequestType type, return ret; } - -/* - * In archive recovery, we rely on checkpointer to do fsyncs, but we will have - * already created the pendingOps during initialization of the startup - * process. Calling this function drops the local pendingOps so that - * subsequent requests will be forwarded to checkpointer. - */ -void -EnableSyncRequestForwarding(void) -{ - /* Perform any pending fsyncs we may have queued up, then drop table */ - if (pendingOps) - { - ProcessSyncRequests(); - hash_destroy(pendingOps); - } - pendingOps = NULL; - - /* - * We should not have any pending unlink requests, since mdunlink doesn't - * queue unlink requests when isRedo. - */ - Assert(pendingUnlinks == NIL); -} diff --git a/src/include/storage/sync.h b/src/include/storage/sync.h index fbdf34f7620..6fd50cfa7b7 100644 --- a/src/include/storage/sync.h +++ b/src/include/storage/sync.h @@ -60,7 +60,6 @@ extern void SyncPreCheckpoint(void); extern void SyncPostCheckpoint(void); extern void ProcessSyncRequests(void); extern void RememberSyncRequest(const FileTag *ftag, SyncRequestType type); -extern void EnableSyncRequestForwarding(void); extern bool RegisterSyncRequest(const FileTag *ftag, SyncRequestType type, bool retryOnError); -- 2.39.5