On Fri, Sep 30, 2022 at 05:04:43PM +1300, Thomas Munro wrote:
> Please go ahead! One thing I had in my notes to check with this patch
> is whether there might be a bug due to computing all times in
> microseconds, but sleeping for a number of milliseconds without
> rounding up (what I mean is that if it's trying to sleep for 900
> microseconds, it might finish up sleeping for 0 milliseconds in a
> tight loop a few times).
I think that's right. If 'next_wakeup - now' is less than 1000, 'nap' will
be 0. My first instinct is to simply round to the nearest millisecond, but
this might still result in wakeups before the calculated wakeup time, and I
think we ultimately want to nap until >= the wakeup time whenever possible
to avoid tight loops. So, I agree that it would be better to round up to
the next millisecond whenever nextWakeup > now. Given the current behavior
is to wake up every 100 milliseconds, an extra millisecond here and there
doesn't seem like it should cause any problems. And I suppose we should
assume we might nap longer already, anyway.
I do see that the wakeup time for WALRCV_WAKEUP_PING might be set to 'now'
whenever wal_receiver_timeout is 1 millisecond, but I think that's existing
behavior, and it feels like an extreme case that we probably needn't worry
about too much.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com