On Fri, Nov 18, 2022 at 12:42 AM Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, Nov 17, 2022 at 2:22 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > Duplication is a problem that I agree with and I have an idea here - > > how about introducing a new function, say EnableStandbyMode() that > > sets StandbyMode to true and disables the startup progress timeout, > > something like the attached? > > That works for me, more or less. But I think that > enable_startup_progress_timeout should be amended to either say if > (log_startup_progress_interval == 0 || StandbyMode) return; or else it > should at least Assert(!StandbyMode), so that we can't accidentally > re-enable the timer after we shut it off.
Hm, an assertion may not help in typical production servers running on non-assert builds. I've modified the if condition, please see the attached v5 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
From 758d4cc81b1c7ed087969df07a958e92a374ee0f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Date: Fri, 18 Nov 2022 09:53:15 +0000 Subject: [PATCH v5] Disable STARTUP_PROGRESS_TIMEOUT in standby mode In standby mode, we actually don't report progress of recovery to not bloat server logs as the standby is always in recovery unless promoted. However, startup_progress_timeout_handler() gets called every log_startup_progress_interval seconds, which is unnecessary. Therefore, we disable the timeout in standby mode. Reported-by: Thomas Munro Authors: Bharath Rupireddy, Simon Riggs Reviewed-by: Robert Haas Backpatch-through: 15 Discussion: https://www.postgresql.org/message-id/CA%2BhUKGKCHSffAj8zZJKJvNX7ygnQFxVD6wm1d-2j3fVw%2BMafPQ%40mail.gmail.com --- src/backend/access/transam/xlogrecovery.c | 21 ++++++++++++--- src/backend/postmaster/startup.c | 33 +++++++++++++++++++---- src/include/postmaster/startup.h | 2 ++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index cb07694aea..2feae1ebd3 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -386,6 +386,7 @@ static bool recoveryStopAfter; /* prototypes for local functions */ static void ApplyWalRecord(XLogReaderState *xlogreader, XLogRecord *record, TimeLineID *replayTLI); +static void EnableStandbyMode(void); static void readRecoverySignalFile(void); static void validateRecoveryParameters(void); static bool read_backup_label(XLogRecPtr *checkPointLoc, @@ -470,6 +471,20 @@ XLogRecoveryShmemInit(void) ConditionVariableInit(&XLogRecoveryCtl->recoveryNotPausedCV); } +static void +EnableStandbyMode(void) +{ + StandbyMode = true; + + /* + * To avoid server log bloat, we don't report recovery progress in a + * standby as it will always be in recovery unless promoted. We disable + * startup progress timeout in standby mode to avoid calling + * startup_progress_timeout_handler() unnecessarily. + */ + disable_startup_progress_timeout(); +} + /* * Prepare the system for WAL recovery, if needed. * @@ -603,7 +618,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, */ InArchiveRecovery = true; if (StandbyModeRequested) - StandbyMode = true; + EnableStandbyMode(); /* * When a backup_label file is present, we want to roll forward from @@ -740,7 +755,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr, { InArchiveRecovery = true; if (StandbyModeRequested) - StandbyMode = true; + EnableStandbyMode(); } /* Get the last valid checkpoint record. */ @@ -3115,7 +3130,7 @@ ReadRecord(XLogPrefetcher *xlogprefetcher, int emode, (errmsg_internal("reached end of WAL in pg_wal, entering archive recovery"))); InArchiveRecovery = true; if (StandbyModeRequested) - StandbyMode = true; + EnableStandbyMode(); SwitchIntoArchiveRecovery(xlogreader->EndRecPtr, replayTLI); minRecoveryPoint = xlogreader->EndRecPtr; diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index f99186eab7..4a46d2070c 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -314,20 +314,29 @@ startup_progress_timeout_handler(void) startup_progress_timer_expired = true; } +void +disable_startup_progress_timeout(void) +{ + /* Quick exit if feature is disabled. */ + if (log_startup_progress_interval == 0) + return; + + disable_timeout(STARTUP_PROGRESS_TIMEOUT, false); + startup_progress_timer_expired = false; +} + /* * Set the start timestamp of the current operation and enable the timeout. */ void -begin_startup_progress_phase(void) +enable_startup_progress_timeout(void) { TimestampTz fin_time; - /* Feature is disabled. */ - if (log_startup_progress_interval == 0) + /* Quick exit if feature is disabled or we're in standby mode */ + if (log_startup_progress_interval == 0 || StandbyMode) return; - disable_timeout(STARTUP_PROGRESS_TIMEOUT, false); - startup_progress_timer_expired = false; startup_progress_phase_start_time = GetCurrentTimestamp(); fin_time = TimestampTzPlusMilliseconds(startup_progress_phase_start_time, log_startup_progress_interval); @@ -335,6 +344,20 @@ begin_startup_progress_phase(void) log_startup_progress_interval); } +/* + * A thin wrapper to first disable and then enable the startup progress timeout. + */ +void +begin_startup_progress_phase(void) +{ + /* Quick exit if feature is disabled. */ + if (log_startup_progress_interval == 0) + return; + + disable_startup_progress_timeout(); + enable_startup_progress_timeout(); +} + /* * Report whether startup progress timeout has occurred. Reset the timer flag * if it did, set the elapsed time to the out parameters and return true, diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h index d66ec1fcb1..12897531c3 100644 --- a/src/include/postmaster/startup.h +++ b/src/include/postmaster/startup.h @@ -32,6 +32,8 @@ extern void PostRestoreCommand(void); extern bool IsPromoteSignaled(void); extern void ResetPromoteSignaled(void); +extern void enable_startup_progress_timeout(void); +extern void disable_startup_progress_timeout(void); extern void begin_startup_progress_phase(void); extern void startup_progress_timeout_handler(void); extern bool has_startup_progress_timeout_expired(long *secs, int *usecs); -- 2.34.1
