On Thu, Nov 17, 2022 at 12:21 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Nov 16, 2022 at 1:47 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> > 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.
>
> Oh, interesting. I didn't realize that we would need to worry about that case.
>
> > I'm attaching the v3 patch for further review. Please find the CF
> > entry here - https://commitfest.postgresql.org/41/4012/.
>
> I kind of dislike having to have logic for this in two places. Seems
> like it could create future bugs.
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?
> How about the attached approach, instead? This way, the first time the
> timer expires after we reach standby mode, we reactively disable it.
Hm. I'm not really sure if it's a good idea. While it simplifies the
code, the has_startup_progress_timeout_expired() gets called for every
WAL record in standby mode. Isn't this an unnecessary thing?
Currently, the if (!StandbyMode) condition blocks the function calls.
And I'm also a little concerned that we move the StandbyMode variable
to startup.c which so far tiled to xlogrecovery.c. Maybe these are not
really concerns at all. Maybe others are okay with this approach.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
