From afce7908d7062d94ac60fd4de5f98aaed134c2c7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 10 Nov 2020 22:51:19 -0500 Subject: Fix and simplify some usages of TimestampDifference(). Introduce TimestampDifferenceMilliseconds() to simplify callers that would rather have the difference in milliseconds, instead of the select()-oriented seconds-and-microseconds format. This gets rid of at least one integer division per call, and it eliminates some apparently-easy-to-mess-up arithmetic. Two of these call sites were in fact wrong: * pg_prewarm's autoprewarm_main() forgot to multiply the seconds by 1000, thus ending up with a delay 1000X shorter than intended. That doesn't quite make it a busy-wait, but close. * postgres_fdw's pgfdw_get_cleanup_result() thought it needed to compute microseconds not milliseconds, thus ending up with a delay 1000X longer than intended. Somebody along the way had noticed this problem but misdiagnosed the cause, and imposed an ad-hoc 60-second limit rather than fixing the units. This was relatively harmless in context, because we don't care that much about exactly how long this delay is; still, it's wrong. There are a few more callers of TimestampDifference() that don't have a direct need for seconds-and-microseconds, but can't use TimestampDifferenceMilliseconds() either because they do need microsecond precision or because they might possibly deal with intervals long enough to overflow 32-bit milliseconds. It might be worth inventing another API to improve that, but that seems outside the scope of this patch; so those callers are untouched here. Given the fact that we are fixing some bugs, and the likelihood that future patches might want to back-patch code that uses this new API, back-patch to all supported branches. Alexey Kondratov and Tom Lane Discussion: https://postgr.es/m/3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru --- src/backend/utils/adt/timestamp.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) (limited to 'src/backend/utils/adt/timestamp.c') diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 0159e05209f..aaab84aedd1 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -1637,12 +1637,14 @@ timeofday(PG_FUNCTION_ARGS) * TimestampDifference -- convert the difference between two timestamps * into integer seconds and microseconds * + * This is typically used to calculate a wait timeout for select(2), + * which explains the otherwise-odd choice of output format. + * * Both inputs must be ordinary finite timestamps (in current usage, * they'll be results from GetCurrentTimestamp()). * - * We expect start_time <= stop_time. If not, we return zeros; for current - * callers there is no need to be tense about which way division rounds on - * negative inputs. + * We expect start_time <= stop_time. If not, we return zeros, + * since then we're already past the previously determined stop_time. */ void TimestampDifference(TimestampTz start_time, TimestampTz stop_time, @@ -1662,6 +1664,36 @@ TimestampDifference(TimestampTz start_time, TimestampTz stop_time, } } +/* + * TimestampDifferenceMilliseconds -- convert the difference between two + * timestamps into integer milliseconds + * + * 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"). + * + * Both inputs must be ordinary finite timestamps (in current usage, + * they'll be results from GetCurrentTimestamp()). + * + * We expect start_time <= stop_time. If not, we return zero, + * since then we're already past the previously determined stop_time. + * + * Note we round up any fractional millisecond, since waiting for just + * less than the intended timeout is undesirable. + */ +long +TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time) +{ + TimestampTz diff = stop_time - start_time; + + if (diff <= 0) + return 0; + else + return (long) ((diff + 999) / 1000); +} + /* * TimestampDifferenceExceeds -- report whether the difference between two * timestamps is >= a threshold (expressed in milliseconds) -- cgit v1.2.3