On Wed, Nov 16, 2022 at 2:28 PM Simon Riggs
<simon.riggs@enterprisedb.com> wrote:
>
> On Wed, 16 Nov 2022 at 06:47, Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Tue, Nov 15, 2022 at 10:55 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > >
> > > On Tue, Nov 15, 2022 at 8:33 AM Bharath Rupireddy
> > > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > > > Please review the v2 patch.
> > >
> > > It seems to me that this will call disable_startup_progress_timeout
> > > once per WAL record, which seems like an unnecessary expense. How
> > > about leaving the code inside the loop just as we have it, and putting
> > > if (StandbyMode) disable_startup_progress_timeout() before entering
> > > the loop?
> >
> > That can be done, only if we can disable the timeout in another place
> > when the StandbyMode is set to true in ReadRecord(), that is, after
> > the standby server finishes crash recovery and enters standby mode.
> >
> > I'm attaching the v3 patch for further review. Please find the CF
> > entry here - https://commitfest.postgresql.org/41/4012/.
>
> begin_startup_progress_phase() checks to see if feature is disabled
> twice, so I think you can skip the check and just rely on the check in
> enable().
Yes, I did that intentionally to avoid begin_startup_progress_phase()
calling disable and enable functions when the feature is disabled.
I'll leave it to the committer whether to retain it or delete it.
> Otherwise, all good.
Thanks.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
