Re: when the startup process doesn't (logging startup delays)
Simon Riggs <simon.riggs@enterprisedb.com>
From: Simon Riggs <simon.riggs@enterprisedb.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>,
Robert Haas <robertmhaas@gmail.com>, Nitin Jadhav <nitinjadhavpostgres@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>,
Justin Pryzby <pryzby@telsasoft.com>, Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>, Stephen Frost <sfrost@snowman.net>, Magnus Hagander <magnus@hagander.net>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-11-14T12:37:27Z
Lists: pgsql-hackers
On Tue, 8 Nov 2022 at 12:33, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > On Tue, Nov 8, 2022 at 4:35 PM Thomas Munro <thomas.munro@gmail.com> wrote: > > > > On Sat, Oct 30, 2021 at 7:44 AM Robert Haas <robertmhaas@gmail.com> wrote: > > > Committed. > > > > Is it expected that an otherwise idle standby's recovery process > > receives SIGALRM every N seconds, or should the timer be canceled at > > that point, as there is no further progress to report? > > Nice catch. Yeah, that seems unnecessary, see the below standby logs. > I think we need to disable_timeout(STARTUP_PROGRESS_TIMEOUT, false);, > something like the attached? I think there'll be no issue with the > patch since the StandbyMode gets reset only at the end of recovery (in > FinishWalRecovery()) but it can very well be set during recovery (in > ReadRecord()). Note that I also added an assertion in > has_startup_progress_timeout_expired(), just in case. > > 2022-11-08 11:28:23.563 UTC [980909] LOG: SIGALRM handle_sig_alarm received > 2022-11-08 11:28:23.563 UTC [980909] LOG: > startup_progress_timeout_handler called > 2022-11-08 11:28:33.563 UTC [980909] LOG: SIGALRM handle_sig_alarm received > 2022-11-08 11:28:33.563 UTC [980909] LOG: > startup_progress_timeout_handler called > 2022-11-08 11:28:43.563 UTC [980909] LOG: SIGALRM handle_sig_alarm received > 2022-11-08 11:28:43.563 UTC [980909] LOG: > startup_progress_timeout_handler called > 2022-11-08 11:28:53.563 UTC [980909] LOG: SIGALRM handle_sig_alarm received > 2022-11-08 11:28:53.563 UTC [980909] LOG: > startup_progress_timeout_handler called > > Whilte at it, I noticed that we report redo progress for PITR, but we > don't report when standby enters archive recovery mode, say due to a > failure in the connection to primary or after the promote signal is > found. Isn't it useful to report in this case as well to know the > recovery progress? I think your patch disables progress too early, effectively turning off the standby progress feature. The purpose was to report on things that take long periods during recovery, not just prior to recovery. I would advocate that we disable progress only while waiting, as I've done here: https://www.postgresql.org/message-id/CANbhV-GcWjZ2cmj0uCbZDWQUHnneMi_4EfY3dVWq0-yD5o7Ccg%40mail.gmail.com -- Simon Riggs http://www.EnterpriseDB.com/
Commits
-
Un-revert "Disable STARTUP_PROGRESS_TIMEOUT in standby mode."
- ecb01e6ebb5a 15.3 landed
-
Revert "Disable STARTUP_PROGRESS_TIMEOUT in standby mode."
- 1eadfbdd7eb0 15.2 landed
-
Disable STARTUP_PROGRESS_TIMEOUT in standby mode.
- 98e7234242a6 15.2 landed
- 8a2f783cc489 16.0 landed
-
Fix race condition in startup progress reporting.
- 5ccceb2946d4 15.0 landed
-
Report progress of startup operations that take a long time.
- 9ce346eabf35 15.0 landed
-
Add enable_timeout_every() to fire the same timeout repeatedly.
- 732e6677a667 15.0 landed