Re: when the startup process doesn't (logging startup delays)
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Simon Riggs <simon.riggs@enterprisedb.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-17T07:22:46Z
Lists: pgsql-hackers
Attachments
- v4-0001-Disable-STARTUP_PROGRESS_TIMEOUT-in-standby-mode.patch (application/x-patch) patch v4-0001
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
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