Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3a28d78

Browse files
committed
Improve TimestampDifferenceMilliseconds to cope with overflow sanely.
We'd like to use TimestampDifferenceMilliseconds with the stop_timepossibly being TIMESTAMP_INFINITY, but up to now it's disclaimedresponsibility for overflow cases. Define it to clamp its output tothe range [0, INT_MAX], handling overflow correctly. (INT_MAX ratherthan LONG_MAX seems appropriate, because the function is alreadydescribed as being intended for calculating wait times for WaitLatchet al, and that infrastructure only handles waits up to INT_MAX.Also, this choice gets rid of cross-platform behavioral differences.)Having done that, we can replace some ad-hoc code in walreceiver.cwith a simple call to TimestampDifferenceMilliseconds.While at it, fix some buglets in existing callers ofTimestampDifferenceMilliseconds: basebackup_copy.c had not read thememo about TimestampDifferenceMilliseconds never returning a negativevalue, and postmaster.c had not read the memo about Min() and Max()being macros with multiple-evaluation hazards. Neither of thesequite seem worth back-patching.Patch by me; thanks to Nathan Bossart for review.Discussion:https://postgr.es/m/3126727.1674759248@sss.pgh.pa.us
1 parent24ff700 commit3a28d78

File tree

4 files changed

+24
-23
lines changed

4 files changed

+24
-23
lines changed

‎src/backend/backup/basebackup_copy.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ bbsink_copystream_archive_contents(bbsink *sink, size_t len)
215215
* the system clock was set backward, so that such occurrences don't
216216
* have the effect of suppressing further progress messages.
217217
*/
218-
if (ms<0||ms >=PROGRESS_REPORT_MILLISECOND_THRESHOLD)
218+
if (ms >=PROGRESS_REPORT_MILLISECOND_THRESHOLD||
219+
now<mysink->last_progress_report_time)
219220
{
220221
mysink->last_progress_report_time=now;
221222

‎src/backend/postmaster/postmaster.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,11 +1670,12 @@ DetermineSleepTime(void)
16701670

16711671
if (next_wakeup!=0)
16721672
{
1673-
/* Ensure we don't exceed one minute, or go under 0. */
1674-
returnMax(0,
1675-
Min(60*1000,
1676-
TimestampDifferenceMilliseconds(GetCurrentTimestamp(),
1677-
next_wakeup)));
1673+
intms;
1674+
1675+
/* result of TimestampDifferenceMilliseconds is in [0, INT_MAX] */
1676+
ms= (int)TimestampDifferenceMilliseconds(GetCurrentTimestamp(),
1677+
next_wakeup);
1678+
returnMin(60*1000,ms);
16781679
}
16791680

16801681
return60*1000;

‎src/backend/replication/walreceiver.c

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ WalReceiverMain(void)
445445
pgsocketwait_fd=PGINVALID_SOCKET;
446446
intrc;
447447
TimestampTznextWakeup;
448-
intnap;
448+
longnap;
449449

450450
/*
451451
* Exit walreceiver if we're not in recovery. This should not
@@ -528,15 +528,9 @@ WalReceiverMain(void)
528528
for (inti=0;i<NUM_WALRCV_WAKEUPS;++i)
529529
nextWakeup=Min(wakeup[i],nextWakeup);
530530

531-
/*
532-
* Calculate the nap time. WaitLatchOrSocket() doesn't accept
533-
* timeouts longer than INT_MAX milliseconds, so we limit the
534-
* result accordingly. Also, we round up to the next
535-
* millisecond to avoid waking up too early and spinning until
536-
* one of the wakeup times.
537-
*/
531+
/* Calculate the nap time, clamping as necessary. */
538532
now=GetCurrentTimestamp();
539-
nap=(int)Min(INT_MAX,Max(0, (nextWakeup-now+999) /1000));
533+
nap=TimestampDifferenceMilliseconds(now,nextWakeup);
540534

541535
/*
542536
* Ideally we would reuse a WaitEventSet object repeatedly

‎src/backend/utils/adt/timestamp.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,26 +1690,31 @@ TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
16901690
*
16911691
* This is typically used to calculate a wait timeout for WaitLatch()
16921692
* or a related function. The choice of "long" as the result type
1693-
* is to harmonize with that. It is caller's responsibility that the
1694-
* input timestamps not be so far apart as to risk overflow of "long"
1695-
* (which'd happen at about 25 days on machines with 32-bit "long").
1696-
*
1697-
* Both inputs must be ordinary finite timestamps (in current usage,
1698-
* they'll be results from GetCurrentTimestamp()).
1693+
* is to harmonize with that; furthermore, we clamp the result to at most
1694+
* INT_MAX milliseconds, because that's all that WaitLatch() allows.
16991695
*
17001696
* We expect start_time <= stop_time. If not, we return zero,
17011697
* since then we're already past the previously determined stop_time.
17021698
*
1699+
* Subtracting finite and infinite timestamps works correctly, returning
1700+
* zero or INT_MAX as appropriate.
1701+
*
17031702
* Note we round up any fractional millisecond, since waiting for just
17041703
* less than the intended timeout is undesirable.
17051704
*/
17061705
long
17071706
TimestampDifferenceMilliseconds(TimestampTzstart_time,TimestampTzstop_time)
17081707
{
1709-
TimestampTzdiff=stop_time-start_time;
1708+
TimestampTzdiff;
17101709

1711-
if (diff <=0)
1710+
/* Deal with zero or negative elapsed time quickly. */
1711+
if (start_time >=stop_time)
17121712
return0;
1713+
/* To not fail with timestamp infinities, we must detect overflow. */
1714+
if (pg_sub_s64_overflow(stop_time,start_time,&diff))
1715+
return (long)INT_MAX;
1716+
if (diff >= (INT_MAX*INT64CONST(1000)-999))
1717+
return (long)INT_MAX;
17131718
else
17141719
return (long) ((diff+999) /1000);
17151720
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp