Re: when the startup process doesn't (logging startup delays)
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
Nitin Jadhav <nitinjadhavpostgres@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Michael Paquier <michael@paquier.xyz>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
Andres Freund <andres@anarazel.de>, Stephen Frost <sfrost@snowman.net>, Magnus Hagander <magnus@hagander.net>, Thomas Munro <thomas.munro@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-09-30T18:41:42Z
Lists: pgsql-hackers
Attachments
- 0002-Quick-testing-hack.patch (application/octet-stream) patch 0002
- 0001-Add-enable_timeout_every-to-fire-the-same-timeout-re.patch (application/octet-stream) patch 0001
On Wed, Sep 29, 2021 at 5:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I didn't claim there are any other places that could use the feature > *today*. But once we've got one, it seems like there could be more > tomorrow. In any case, I dislike keeping timeout state data outside > timeout.c, because it's so likely to get out-of-sync that way. Well, I had a quick go at implementing this (attached). It seems to do a satisfactory job preventing drift over time, but it doesn't behave nicely if you set the system clock backward. With a bit of extra debugging output: 2021-09-30 14:23:50.291 EDT [2279] LOG: scheduling wakeup in 2 secs, 998727 usecs 2021-09-30 14:23:53.291 EDT [2279] LOG: scheduling wakeup in 2 secs, 998730 usecs 2021-09-30 14:23:56.291 EDT [2279] LOG: scheduling wakeup in 2 secs, 998728 usecs 2021-09-30 14:20:01.154 EDT [2279] LOG: scheduling wakeup in 238 secs, 135532 usecs 2021-09-30 14:23:59.294 EDT [2279] LOG: scheduling wakeup in 2 secs, 995922 use The issue here is that fin_time is really the driving force behind everything timeout.c does. In particular, it determines the order of active_timeouts[]. And that's not really correct either for enable_timeout_after(), or for the new function I added in this draft patch, enable_timeout_every(). When I say I want my handler to be fired in 3 s, I don't mean that I want it to be fired when the system time is 3 seconds greater than it is right now. I mean I want it to be fired in 3 actual seconds, regardless of what dumb thing the system clock may choose to do. I don't really think that precise behavior can be implemented, but ideally if a timeout that was supposed to happen after 3 s is now scheduled for a time that is more than 3 seconds beyond the current value of the system clock, we'd move the firing time backwards to 3 seconds beyond the current system clock. That way, if you set the system time backward by 4 minutes, you might see a few seconds of delay before the next firing, but you wouldn't go into the tank for 4 minutes. I don't see an obvious way of making timeout.c behave like that without major surgery, though. If nobody else does either, then we could (1) stick with something closer to Nitin's current patch, which tries to handle this concern outside of timeout.c, (2) adopt something like the attached 0001 and leave the question of improved behavior in case of backwards system clock adjustments for another day, or (3) undertake to rewrite timeout.c as a precondition of being able to log messages about why startup is slow. I'm not a huge fan of (3) but I'm open to suggestions. -- Robert Haas EDB: 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