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