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: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@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-15T14:27:55Z
Lists: pgsql-hackers
On Tue, 15 Nov 2022 at 13:33, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Mon, Nov 14, 2022 at 9:31 PM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Mon, Nov 14, 2022 at 7:37 AM Simon Riggs
> > <simon.riggs@enterprisedb.com> wrote:
> > > > 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
> >
> > Maybe I'm confused here, but I think that, on a standby, startup
> > progress messages are only printed until the main redo loop is
> > reached. Otherwise, we would print a message on a standby every 10s
> > forever, which seems like a thing that most users would not like. So I
> > think that Bharath has the right idea here.
>
> Yes, the idea is to disable the timeout on standby completely since we
> actually don't report any recovery progress. Keeping it enabled,
> unnecessarily calls startup_progress_timeout_handler() every
> log_startup_progress_interval seconds i.e. 10 seconds. That's the
> intention of the patch.

As long as we don't get the SIGALRMs that Thomas identified, then I'm happy.

-- 
Simon Riggs                http://www.EnterpriseDB.com/



Commits

  1. Un-revert "Disable STARTUP_PROGRESS_TIMEOUT in standby mode."

  2. Revert "Disable STARTUP_PROGRESS_TIMEOUT in standby mode."

  3. Disable STARTUP_PROGRESS_TIMEOUT in standby mode.

  4. Fix race condition in startup progress reporting.

  5. Report progress of startup operations that take a long time.

  6. Add enable_timeout_every() to fire the same timeout repeatedly.