Nathan Bossart <nathandboss...@gmail.com> writes: > I think we might risk overflowing "long" when all the wakeup times are > DT_NOEND:
> * This is typically used to calculate a wait timeout for WaitLatch() > * or a related function. The choice of "long" as the result type > * is to harmonize with that. It is caller's responsibility that the > * input timestamps not be so far apart as to risk overflow of "long" > * (which'd happen at about 25 days on machines with 32-bit "long"). > Maybe we can adjust that function or create a new one to deal with this. It'd probably be reasonable to file down that sharp edge by instead specifying that TimestampDifferenceMilliseconds will clamp overflowing differences to LONG_MAX. Maybe there should be a clamp on the underflow side too ... but should it be to LONG_MIN or to zero? BTW, as long as we're discussing roundoff gotchas, I noticed while testing your previous patch that there's some inconsistency between TimestampDifferenceExceeds and TimestampDifferenceMilliseconds. What you submitted at [1] did this: + if (TimestampDifferenceExceeds(last_start, now, + wal_retrieve_retry_interval)) + ... + else + { + long elapsed; + + elapsed = TimestampDifferenceMilliseconds(last_start, now); + wait_time = Min(wait_time, wal_retrieve_retry_interval - elapsed); + } and I discovered that that could sometimes busy-wait by repeatedly falling through to the "else", but then calculating elapsed == wal_retrieve_retry_interval and hence setting wait_time to zero. I fixed it in the committed version [2] by always computing "elapsed" and then checking if that's strictly less than wal_retrieve_retry_interval, but I bet there's existing code with the same issue. I think we need to take a closer look at making TimestampDifferenceMilliseconds' roundoff behavior match the outcome of TimestampDifferenceExceeds comparisons. regards, tom lane [1] https://www.postgresql.org/message-id/20230110174345.GA1292607%40nathanxps13 [2] https://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=5a3a95385