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
Appearance settings

Commit ec29427

Browse filesBrowse files
tglsfdctglsfdc
committed
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
1 parent b8b6a01 commit ec29427
Copy full SHA for ec29427

File tree

Expand file treeCollapse file tree

7 files changed

+80
-99
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+80
-99
lines changed

‎contrib/pg_prewarm/autoprewarm.c

Copy file name to clipboardExpand all lines: contrib/pg_prewarm/autoprewarm.c
+5-7Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,18 +220,16 @@ autoprewarm_main(Datum main_arg)
220220
}
221221
else
222222
{
223-
long delay_in_ms = 0;
224-
TimestampTz next_dump_time = 0;
225-
long secs = 0;
226-
int usecs = 0;
223+
TimestampTz next_dump_time;
224+
long delay_in_ms;
227225

228226
/* Compute the next dump time. */
229227
next_dump_time =
230228
TimestampTzPlusMilliseconds(last_dump_time,
231229
autoprewarm_interval * 1000);
232-
TimestampDifference(GetCurrentTimestamp(), next_dump_time,
233-
&secs, &usecs);
234-
delay_in_ms = secs + (usecs / 1000);
230+
delay_in_ms =
231+
TimestampDifferenceMilliseconds(GetCurrentTimestamp(),
232+
next_dump_time);
235233

236234
/* Perform a dump if it's time. */
237235
if (delay_in_ms <= 0)

‎contrib/postgres_fdw/connection.c

Copy file name to clipboardExpand all lines: contrib/postgres_fdw/connection.c
+2-7Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,20 +1271,15 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result)
12711271
{
12721272
int wc;
12731273
TimestampTz now = GetCurrentTimestamp();
1274-
long secs;
1275-
int microsecs;
12761274
long cur_timeout;
12771275

12781276
/* If timeout has expired, give up, else get sleep time. */
1279-
if (now >= endtime)
1277+
cur_timeout = TimestampDifferenceMilliseconds(now, endtime);
1278+
if (cur_timeout <= 0)
12801279
{
12811280
timed_out = true;
12821281
goto exit;
12831282
}
1284-
TimestampDifference(now, endtime, &secs, &microsecs);
1285-
1286-
/* To protect against clock skew, limit sleep to one minute. */
1287-
cur_timeout = Min(60000, secs * USECS_PER_SEC + microsecs);
12881283

12891284
/* Sleep until there's something to do */
12901285
wc = WaitLatchOrSocket(MyLatch,

‎src/backend/access/transam/xlog.c

Copy file name to clipboardExpand all lines: src/backend/access/transam/xlog.c
+31-54Lines changed: 31 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6074,8 +6074,7 @@ recoveryApplyDelay(XLogReaderState *record)
60746074
uint8 xact_info;
60756075
TimestampTz xtime;
60766076
TimestampTz delayUntil;
6077-
long secs;
6078-
int microsecs;
6077+
long msecs;
60796078

60806079
/* nothing to do if no delay configured */
60816080
if (recovery_min_apply_delay <= 0)
@@ -6115,8 +6114,8 @@ recoveryApplyDelay(XLogReaderState *record)
61156114
* Exit without arming the latch if it's already past time to apply this
61166115
* record
61176116
*/
6118-
TimestampDifference(GetCurrentTimestamp(), delayUntil, &secs, &microsecs);
6119-
if (secs <= 0 && microsecs <= 0)
6117+
msecs = TimestampDifferenceMilliseconds(GetCurrentTimestamp(), delayUntil);
6118+
if (msecs <= 0)
61206119
return false;
61216120

61226121
while (true)
@@ -6132,22 +6131,17 @@ recoveryApplyDelay(XLogReaderState *record)
61326131
/*
61336132
* Wait for difference between GetCurrentTimestamp() and delayUntil
61346133
*/
6135-
TimestampDifference(GetCurrentTimestamp(), delayUntil,
6136-
&secs, &microsecs);
6134+
msecs = TimestampDifferenceMilliseconds(GetCurrentTimestamp(),
6135+
delayUntil);
61376136

6138-
/*
6139-
* NB: We're ignoring waits below recovery_min_apply_delay's
6140-
* resolution.
6141-
*/
6142-
if (secs <= 0 && microsecs / 1000 <= 0)
6137+
if (msecs <= 0)
61436138
break;
61446139

6145-
elog(DEBUG2, "recovery apply delay %ld seconds, %d milliseconds",
6146-
secs, microsecs / 1000);
6140+
elog(DEBUG2, "recovery apply delay %ld milliseconds", msecs);
61476141

61486142
(void) WaitLatch(MyLatch,
61496143
WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
6150-
secs * 1000L + microsecs / 1000,
6144+
msecs,
61516145
WAIT_EVENT_RECOVERY_APPLY_DELAY);
61526146
}
61536147
return true;
@@ -8555,33 +8549,24 @@ LogCheckpointStart(int flags, bool restartpoint)
85558549
static void
85568550
LogCheckpointEnd(bool restartpoint)
85578551
{
8558-
long write_secs,
8559-
sync_secs,
8560-
total_secs,
8561-
longest_secs,
8562-
average_secs;
8563-
int write_usecs,
8564-
sync_usecs,
8565-
total_usecs,
8566-
longest_usecs,
8567-
average_usecs;
8552+
long write_msecs,
8553+
sync_msecs,
8554+
total_msecs,
8555+
longest_msecs,
8556+
average_msecs;
85688557
uint64 average_sync_time;
85698558

85708559
CheckpointStats.ckpt_end_t = GetCurrentTimestamp();
85718560

8572-
TimestampDifference(CheckpointStats.ckpt_write_t,
8573-
CheckpointStats.ckpt_sync_t,
8574-
&write_secs, &write_usecs);
8561+
write_msecs = TimestampDifferenceMilliseconds(CheckpointStats.ckpt_write_t,
8562+
CheckpointStats.ckpt_sync_t);
85758563

8576-
TimestampDifference(CheckpointStats.ckpt_sync_t,
8577-
CheckpointStats.ckpt_sync_end_t,
8578-
&sync_secs, &sync_usecs);
8564+
sync_msecs = TimestampDifferenceMilliseconds(CheckpointStats.ckpt_sync_t,
8565+
CheckpointStats.ckpt_sync_end_t);
85798566

85808567
/* Accumulate checkpoint timing summary data, in milliseconds. */
8581-
BgWriterStats.m_checkpoint_write_time +=
8582-
write_secs * 1000 + write_usecs / 1000;
8583-
BgWriterStats.m_checkpoint_sync_time +=
8584-
sync_secs * 1000 + sync_usecs / 1000;
8568+
BgWriterStats.m_checkpoint_write_time += write_msecs;
8569+
BgWriterStats.m_checkpoint_sync_time += sync_msecs;
85858570

85868571
/*
85878572
* All of the published timing statistics are accounted for. Only
@@ -8590,25 +8575,20 @@ LogCheckpointEnd(bool restartpoint)
85908575
if (!log_checkpoints)
85918576
return;
85928577

8593-
TimestampDifference(CheckpointStats.ckpt_start_t,
8594-
CheckpointStats.ckpt_end_t,
8595-
&total_secs, &total_usecs);
8578+
total_msecs = TimestampDifferenceMilliseconds(CheckpointStats.ckpt_start_t,
8579+
CheckpointStats.ckpt_end_t);
85968580

85978581
/*
85988582
* Timing values returned from CheckpointStats are in microseconds.
8599-
* Convert to the second plus microsecond form that TimestampDifference
8600-
* returns for homogeneous printing.
8583+
* Convert to milliseconds for consistent printing.
86018584
*/
8602-
longest_secs = (long) (CheckpointStats.ckpt_longest_sync / 1000000);
8603-
longest_usecs = CheckpointStats.ckpt_longest_sync -
8604-
(uint64) longest_secs * 1000000;
8585+
longest_msecs = (long) ((CheckpointStats.ckpt_longest_sync + 999) / 1000);
86058586

86068587
average_sync_time = 0;
86078588
if (CheckpointStats.ckpt_sync_rels > 0)
86088589
average_sync_time = CheckpointStats.ckpt_agg_sync_time /
86098590
CheckpointStats.ckpt_sync_rels;
8610-
average_secs = (long) (average_sync_time / 1000000);
8611-
average_usecs = average_sync_time - (uint64) average_secs * 1000000;
8591+
average_msecs = (long) ((average_sync_time + 999) / 1000);
86128592

86138593
elog(LOG, "%s complete: wrote %d buffers (%.1f%%); "
86148594
"%d WAL file(s) added, %d removed, %d recycled; "
@@ -8621,12 +8601,12 @@ LogCheckpointEnd(bool restartpoint)
86218601
CheckpointStats.ckpt_segs_added,
86228602
CheckpointStats.ckpt_segs_removed,
86238603
CheckpointStats.ckpt_segs_recycled,
8624-
write_secs, write_usecs / 1000,
8625-
sync_secs, sync_usecs / 1000,
8626-
total_secs, total_usecs / 1000,
8604+
write_msecs / 1000, (int) (write_msecs % 1000),
8605+
sync_msecs / 1000, (int) (sync_msecs % 1000),
8606+
total_msecs / 1000, (int) (total_msecs % 1000),
86278607
CheckpointStats.ckpt_sync_rels,
8628-
longest_secs, longest_usecs / 1000,
8629-
average_secs, average_usecs / 1000,
8608+
longest_msecs / 1000, (int) (longest_msecs % 1000),
8609+
average_msecs / 1000, (int) (average_msecs % 1000),
86308610
(int) (PrevCheckPointDistance / 1024.0),
86318611
(int) (CheckPointDistanceEstimate / 1024.0));
86328612
}
@@ -12233,13 +12213,10 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
1223312213
if (!TimestampDifferenceExceeds(last_fail_time, now,
1223412214
wal_retrieve_retry_interval))
1223512215
{
12236-
long secs,
12237-
wait_time;
12238-
int usecs;
12216+
long wait_time;
1223912217

12240-
TimestampDifference(last_fail_time, now, &secs, &usecs);
1224112218
wait_time = wal_retrieve_retry_interval -
12242-
(secs * 1000 + usecs / 1000);
12219+
TimestampDifferenceMilliseconds(last_fail_time, now);
1224312220

1224412221
(void) WaitLatch(MyLatch,
1224512222
WL_LATCH_SET | WL_TIMEOUT |

‎src/backend/replication/walreceiverfuncs.c

Copy file name to clipboardExpand all lines: src/backend/replication/walreceiverfuncs.c
+4-21Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,6 @@ GetReplicationApplyDelay(void)
350350
WalRcvData *walrcv = WalRcv;
351351
XLogRecPtr receivePtr;
352352
XLogRecPtr replayPtr;
353-
354-
long secs;
355-
int usecs;
356-
357353
TimestampTz chunkReplayStartTime;
358354

359355
SpinLockAcquire(&walrcv->mutex);
@@ -370,11 +366,8 @@ GetReplicationApplyDelay(void)
370366
if (chunkReplayStartTime == 0)
371367
return -1;
372368

373-
TimestampDifference(chunkReplayStartTime,
374-
GetCurrentTimestamp(),
375-
&secs, &usecs);
376-
377-
return (((int) secs * 1000) + (usecs / 1000));
369+
return TimestampDifferenceMilliseconds(chunkReplayStartTime,
370+
GetCurrentTimestamp());
378371
}
379372

380373
/*
@@ -385,24 +378,14 @@ int
385378
GetReplicationTransferLatency(void)
386379
{
387380
WalRcvData *walrcv = WalRcv;
388-
389381
TimestampTz lastMsgSendTime;
390382
TimestampTz lastMsgReceiptTime;
391383

392-
long secs = 0;
393-
int usecs = 0;
394-
int ms;
395-
396384
SpinLockAcquire(&walrcv->mutex);
397385
lastMsgSendTime = walrcv->lastMsgSendTime;
398386
lastMsgReceiptTime = walrcv->lastMsgReceiptTime;
399387
SpinLockRelease(&walrcv->mutex);
400388

401-
TimestampDifference(lastMsgSendTime,
402-
lastMsgReceiptTime,
403-
&secs, &usecs);
404-
405-
ms = ((int) secs * 1000) + (usecs / 1000);
406-
407-
return ms;
389+
return TimestampDifferenceMilliseconds(lastMsgSendTime,
390+
lastMsgReceiptTime);
408391
}

‎src/backend/replication/walsender.c

Copy file name to clipboardExpand all lines: src/backend/replication/walsender.c
+1-7Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2194,8 +2194,6 @@ WalSndComputeSleeptime(TimestampTz now)
21942194
if (wal_sender_timeout > 0 && last_reply_timestamp > 0)
21952195
{
21962196
TimestampTz wakeup_time;
2197-
long sec_to_timeout;
2198-
int microsec_to_timeout;
21992197

22002198
/*
22012199
* At the latest stop sleeping once wal_sender_timeout has been
@@ -2214,11 +2212,7 @@ WalSndComputeSleeptime(TimestampTz now)
22142212
wal_sender_timeout / 2);
22152213

22162214
/* Compute relative time until wakeup. */
2217-
TimestampDifference(now, wakeup_time,
2218-
&sec_to_timeout, &microsec_to_timeout);
2219-
2220-
sleeptime = sec_to_timeout * 1000 +
2221-
microsec_to_timeout / 1000;
2215+
sleeptime = TimestampDifferenceMilliseconds(now, wakeup_time);
22222216
}
22232217

22242218
return sleeptime;

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/timestamp.c
+35-3Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,12 +1641,14 @@ timeofday(PG_FUNCTION_ARGS)
16411641
* TimestampDifference -- convert the difference between two timestamps
16421642
* into integer seconds and microseconds
16431643
*
1644+
* This is typically used to calculate a wait timeout for select(2),
1645+
* which explains the otherwise-odd choice of output format.
1646+
*
16441647
* Both inputs must be ordinary finite timestamps (in current usage,
16451648
* they'll be results from GetCurrentTimestamp()).
16461649
*
1647-
* We expect start_time <= stop_time. If not, we return zeros; for current
1648-
* callers there is no need to be tense about which way division rounds on
1649-
* negative inputs.
1650+
* We expect start_time <= stop_time. If not, we return zeros,
1651+
* since then we're already past the previously determined stop_time.
16501652
*/
16511653
void
16521654
TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
@@ -1666,6 +1668,36 @@ TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
16661668
}
16671669
}
16681670

1671+
/*
1672+
* TimestampDifferenceMilliseconds -- convert the difference between two
1673+
* timestamps into integer milliseconds
1674+
*
1675+
* This is typically used to calculate a wait timeout for WaitLatch()
1676+
* or a related function. The choice of "long" as the result type
1677+
* is to harmonize with that. It is caller's responsibility that the
1678+
* input timestamps not be so far apart as to risk overflow of "long"
1679+
* (which'd happen at about 25 days on machines with 32-bit "long").
1680+
*
1681+
* Both inputs must be ordinary finite timestamps (in current usage,
1682+
* they'll be results from GetCurrentTimestamp()).
1683+
*
1684+
* We expect start_time <= stop_time. If not, we return zero,
1685+
* since then we're already past the previously determined stop_time.
1686+
*
1687+
* Note we round up any fractional millisecond, since waiting for just
1688+
* less than the intended timeout is undesirable.
1689+
*/
1690+
long
1691+
TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time)
1692+
{
1693+
TimestampTz diff = stop_time - start_time;
1694+
1695+
if (diff <= 0)
1696+
return 0;
1697+
else
1698+
return (long) ((diff + 999) / 1000);
1699+
}
1700+
16691701
/*
16701702
* TimestampDifferenceExceeds -- report whether the difference between two
16711703
* timestamps is >= a threshold (expressed in milliseconds)

‎src/include/utils/timestamp.h

Copy file name to clipboardExpand all lines: src/include/utils/timestamp.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ extern TimestampTz GetSQLCurrentTimestamp(int32 typmod);
7272
extern Timestamp GetSQLLocalTimestamp(int32 typmod);
7373
extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
7474
long *secs, int *microsecs);
75+
extern long TimestampDifferenceMilliseconds(TimestampTz start_time,
76+
TimestampTz stop_time);
7577
extern bool TimestampDifferenceExceeds(TimestampTz start_time,
7678
TimestampTz stop_time,
7779
int msec);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.