From 7a82c9e622ea45c981d05dff08c1d4279ec3c73b Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 26 Aug 2020 16:34:33 +1200 Subject: [PATCH v2 1/3] Run checkpointer and bgworker in crash recovery. Relieve the startup process of some writeback and checkpointing work during crash recovery, making it more like replication recovery. This wasn't done back in commit cdd46c76 out of caution, but now it seems more conservative to have the crash recovery environment work the same as the more commonly exercised streaming replication environment. In order to have a bgwriter running, you also need a checkpointer. While you could give startup and bgwriter their own backend private pending ops table, it's nicer to merger their requests in one place. Reviewed-by: Tom Lane Reviewed-by: Simon Riggs Reviewed-by: Robert Haas Tested-by: Jakub Wartak Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com --- src/backend/access/transam/xlog.c | 32 +++++++++------------------ 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, 21 insertions(+), 65 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f4d1ce5dea..6e8e6cf1e4 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -880,9 +880,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; @@ -7268,25 +7265,14 @@ StartupXLOG(void) /* Also ensure XLogReceiptTime has a sane value */ XLogReceiptTime = GetCurrentTimestamp(); + 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 @@ -7879,7 +7865,7 @@ StartupXLOG(void) * after we're fully out of recovery mode and already accepting * queries. */ - if (bgwriterLaunched) + if (ArchiveRecoveryRequested && IsUnderPostmaster) { if (LocalPromoteIsTriggered) { @@ -7915,7 +7901,11 @@ StartupXLOG(void) CHECKPOINT_WAIT); } else - CreateCheckPoint(CHECKPOINT_END_OF_RECOVERY | CHECKPOINT_IMMEDIATE); + { + RequestCheckpoint(CHECKPOINT_END_OF_RECOVERY | + CHECKPOINT_IMMEDIATE | + CHECKPOINT_WAIT); + } } if (ArchiveRecoveryRequested) @@ -12123,7 +12113,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 715d5195bb..5584f4bc24 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 5907a7befc..7b7f01e1fc 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 edab95a19e..4d5b4b9775 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1392,6 +1392,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... */ @@ -1754,7 +1760,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(); @@ -5125,15 +5131,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 708215614d..ee18898ea2 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 fbdf34f762..6fd50cfa7b 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.30.1