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

Commitec29427

Browse files
committed
Fix and simplify some usages of TimestampDifference().
Introduce TimestampDifferenceMilliseconds() to simplify callersthat would rather have the difference in milliseconds, instead ofthe select()-oriented seconds-and-microseconds format. This getsrid of at least one integer division per call, and it eliminatessome apparently-easy-to-mess-up arithmetic.Two of these call sites were in fact wrong:* pg_prewarm's autoprewarm_main() forgot to multiply the secondsby 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 computemicroseconds not milliseconds, thus ending up with a delay 1000X longerthan intended. Somebody along the way had noticed this problem butmisdiagnosed the cause, and imposed an ad-hoc 60-second limit ratherthan fixing the units. This was relatively harmless in context, becausewe 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'thave a direct need for seconds-and-microseconds, but can't useTimestampDifferenceMilliseconds() either because they do needmicrosecond precision or because they might possibly deal withintervals long enough to overflow 32-bit milliseconds. It might beworth inventing another API to improve that, but that seems outsidethe scope of this patch; so those callers are untouched here.Given the fact that we are fixing some bugs, and the likelihoodthat future patches might want to back-patch code that uses thisnew API, back-patch to all supported branches.Alexey Kondratov and Tom LaneDiscussion:https://postgr.es/m/3b1c053a21c07c1ed5e00be3b2b855ef@postgrespro.ru
1 parentb8b6a01 commitec29427

File tree

7 files changed

+80
-99
lines changed

7 files changed

+80
-99
lines changed

‎contrib/pg_prewarm/autoprewarm.c

Lines 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-
longdelay_in_ms=0;
224-
TimestampTznext_dump_time=0;
225-
longsecs=0;
226-
intusecs=0;
223+
TimestampTznext_dump_time;
224+
longdelay_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

Lines 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
intwc;
12731273
TimestampTznow=GetCurrentTimestamp();
1274-
longsecs;
1275-
intmicrosecs;
12761274
longcur_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
gotoexit;
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

Lines changed: 31 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -6074,8 +6074,7 @@ recoveryApplyDelay(XLogReaderState *record)
60746074
uint8xact_info;
60756075
TimestampTzxtime;
60766076
TimestampTzdelayUntil;
6077-
longsecs;
6078-
intmicrosecs;
6077+
longmsecs;
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
staticvoid
85568550
LogCheckpointEnd(boolrestartpoint)
85578551
{
8558-
longwrite_secs,
8559-
sync_secs,
8560-
total_secs,
8561-
longest_secs,
8562-
average_secs;
8563-
intwrite_usecs,
8564-
sync_usecs,
8565-
total_usecs,
8566-
longest_usecs,
8567-
average_usecs;
8552+
longwrite_msecs,
8553+
sync_msecs,
8554+
total_msecs,
8555+
longest_msecs,
8556+
average_msecs;
85688557
uint64average_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-
longsecs,
12237-
wait_time;
12238-
intusecs;
12216+
longwait_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

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,6 @@ GetReplicationApplyDelay(void)
350350
WalRcvData*walrcv=WalRcv;
351351
XLogRecPtrreceivePtr;
352352
XLogRecPtrreplayPtr;
353-
354-
longsecs;
355-
intusecs;
356-
357353
TimestampTzchunkReplayStartTime;
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+
returnTimestampDifferenceMilliseconds(chunkReplayStartTime,
370+
GetCurrentTimestamp());
378371
}
379372

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

392-
longsecs=0;
393-
intusecs=0;
394-
intms;
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-
returnms;
389+
returnTimestampDifferenceMilliseconds(lastMsgSendTime,
390+
lastMsgReceiptTime);
408391
}

‎src/backend/replication/walsender.c

Lines 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
TimestampTzwakeup_time;
2197-
longsec_to_timeout;
2198-
intmicrosec_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
returnsleeptime;

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

Lines 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(TimestampTzstart_time,TimestampTzstop_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(TimestampTzstart_time,TimestampTzstop_time)
1692+
{
1693+
TimestampTzdiff=stop_time-start_time;
1694+
1695+
if (diff <=0)
1696+
return0;
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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ extern TimestampTz GetSQLCurrentTimestamp(int32 typmod);
7272
externTimestampGetSQLLocalTimestamp(int32typmod);
7373
externvoidTimestampDifference(TimestampTzstart_time,TimestampTzstop_time,
7474
long*secs,int*microsecs);
75+
externlongTimestampDifferenceMilliseconds(TimestampTzstart_time,
76+
TimestampTzstop_time);
7577
externboolTimestampDifferenceExceeds(TimestampTzstart_time,
7678
TimestampTzstop_time,
7779
intmsec);

0 commit comments

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