8000 pg_stat_wal: Accumulate time as instr_time instead of microseconds · postgres/postgres@ca7b3c4 · GitHub
[go: up one dir, main page]
Skip to content

Commit ca7b3c4

Browse files
anarazelanarazel
committed
pg_stat_wal: Accumulate time as instr_time instead of microseconds
In instr_time.h it is stated that: * When summing multiple measurements, it's recommended to leave the * running sum in instr_time form (ie, use INSTR_TIME_ADD or * INSTR_TIME_ACCUM_DIFF) and convert to a result format only at the end. The reason for that is that converting to microseconds is not cheap, and can loose precision. Therefore this commit changes 'PendingWalStats' to use 'instr_time' instead of 'PgStat_Counter' while accumulating 'wal_write_time' and 'wal_sync_time'. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/1feedb83-7aa9-cb4b-5086-598349d3f555@gmail.com
1 parent 122376f commit ca7b3c4

File tree

5 files changed

+37
-24
lines changed

5 files changed

+37
-24
lines changed

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

Lines changed: 2 additions & 4 deletions
< 8000 td data-grid-cell-id="diff-c1cb3ab2a19606390c1a7ed00ffe5a45531702ca5faf999d401c548f8951c65b-2212-2211-2" data-line-anchor="diff-c1cb3ab2a19606390c1a7ed00ffe5a45531702ca5faf999d401c548f8951c65bR2211" data-selected="false" role="gridcell" style="background-color:var(--bgColor-default);padding-right:24px" tabindex="-1" valign="top" class="focusable-grid-cell diff-text-cell right-side-diff-cell left-side">

Original file line numberDiff line numberDiff line change
@@ -2206,8 +2206,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
22062206
instr_time duration;
22072207

22082208
INSTR_TIME_SET_CURRENT(duration);
2209-
INSTR_TIME_SUBTRACT(duration, start);
2210-
PendingWalStats.wal_write_time += INSTR_TIME_GET_MICROSEC(duration);
2209+
INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_write_time, duration, start);
22112210
}
22122211
22132212
PendingWalStats.wal_write++;
@@ -8204,8 +8203,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
82048203
instr_time duration;
82058204

82068205
INSTR_TIME_SET_CURRENT(duration);
8207-
INSTR_TIME_SUBTRACT(duration, start);
8208-
PendingWalStats.wal_sync_time += INSTR_TIME_GET_MICROSEC(duration);
8206+
INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, duration, start);
82098207
}
82108208

82118209
PendingWalStats.wal_sync++;

‎src/backend/storage/file/buffile.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,7 @@ BufFileLoadBuffer(BufFile *file)
469469
if (track_io_timing)
470470
{
471471
INSTR_TIME_SET_CURRENT(io_time);
472-
INSTR_TIME_SUBTRACT(io_time, io_start);
473 8000 -
INSTR_TIME_ADD(pgBufferUsage.temp_blk_read_time, io_time);
472+
INSTR_TIME_ACCUM_DIFF(pgBufferUsage.temp_blk_read_time, io_time, io_start);
474473
}
475474

476475
/* we choose not to advance curOffset here */
@@ -544,8 +543,7 @@ BufFileDumpBuffer(BufFile *file)
544543
if (track_io_timing)
545544
{
546545
INSTR_TIME_SET_CURRENT(io_time);
547-
INSTR_TIME_SUBTRACT(io_time, io_start);
548-
INSTR_TIME_ADD(pgBufferUsage.temp_blk_write_time, io_time);
546+
INSTR_TIME_ACCUM_DIFF(pgBufferUsage.temp_blk_write_time, io_time, io_start);
549547
}
550548

551549
file->curOffset += bytestowrite;

‎src/backend/utils/activity/pgstat_wal.c

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include "executor/instrument.h"
2222

2323

24-
PgStat_WalStats PendingWalStats = {0};
24+
PgStat_PendingWalStats PendingWalStats = {0};
2525

2626
/*
2727
* WAL usage counters saved from pgWALUsage at the previous call to
@@ -70,7 +70,7 @@ bool
7070
pgstat_flush_wal(bool nowait)
7171
{
7272
PgStatShared_Wal *stats_shmem = &pgStatLocal.shmem->wal;
73-
WalUsage diff = {0};
73+
WalUsage wal_usage_diff = {0};
7474

7575
Assert(IsUnderPostmaster || !IsPostmasterEnvironment);
7676
Assert(pgStatLocal.shmem != NULL &&
@@ -88,25 +88,26 @@ pgstat_flush_wal(bool nowait)
8888
* Calculate how much WAL usage counters were increased by subtracting the
8989
* previous counters from the current ones.
9090
*/
91-
WalUsageAccumDiff(&diff, &pgWalUsage, &prevWalUsage);
92-
PendingWalStats.wal_records = diff.wal_records;
93-
PendingWalStats.wal_fpi = diff.wal_fpi;
94-
PendingWalStats.wal_bytes = diff.wal_bytes;
91+
WalUsageAccumDiff(&wal_usage_diff, &pgWalUsage, &prevWalUsage);
9592

9693
if (!nowait)
9794
LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE);
9895
else if (!LWLockConditionalAcquire(&stats_shmem->lock, LW_EXCLUSIVE))
9996
return true;
10097

101-
#define WALSTAT_ACC(fld) stats_shmem->stats.fld += PendingWalStats.fld
102-
WALSTAT_ACC(wal_records);
103-
WALSTAT_ACC(wal_fpi);
104-
WALSTAT_ACC(wal_bytes);
105-
WALSTAT_ACC(wal_buffers_full);
106-
WALSTAT_ACC(wal_write);
107-
WALSTAT_ACC(wal_sync);
108-
WALSTAT_ACC(wal_write_time);
109-
WALSTAT_ACC(wal_sync_time);
98+
#define WALSTAT_ACC(fld, var_to_add) \
99+
(stats_shmem->stats.fld += var_to_add.fld)
100+
#define WALSTAT_ACC_INSTR_TIME(fld) \
101+
(stats_shmem->stats.fld += INSTR_TIME_GET_MICROSEC(PendingWalStats.fld))
102+
WALSTAT_ACC(wal_records, wal_usage_diff);
103+
WALSTAT_ACC(wal_fpi, wal_usage_diff);
104+
WALSTAT_ACC(wal_bytes, wal_usage_diff);
105+
WALSTAT_ACC(wal_buffers_full, PendingWalStats);
106+
WALSTAT_ACC(wal_write, PendingWalStats);
107+
WALSTAT_ACC(wal_sync, PendingWalStats);
108+
WALSTAT_ACC_INSTR_TIME(wal_write_time);
109+
WALSTAT_ACC_INSTR_TIME(wal_sync_time);
110+
#undef WALSTAT_ACC_INSTR_TIME
110111
#undef WALSTAT_ACC
111112

112113
LWLockRelease(&stats_shmem->lock);

‎src/include/pgstat.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,21 @@ typedef struct PgStat_WalStats
435435
TimestampTz stat_reset_timestamp;
436436
} PgStat_WalStats;
437437

438+
/*
439+
* This struct stores wal-related durations as instr_time, which makes it
440+
* cheaper and easier to accumulate them, by not requiring type
441+
* conversions. During stats flush instr_time will be converted into
442+
* microseconds.
443+
*/
444+
typedef struct PgStat_PendingWalStats
445+
{
446+
PgStat_Counter wal_buffers_full;
447+
PgStat_Counter wal_write;
448+
PgStat_Counter wal_sync;
449+
instr_time wal_write_time;
450+
instr_time wal_sync_time;
451+
} PgStat_PendingWalStats;
452+
438453

439454
/*
440455
* Functions in pgstat.c
@@ -748,7 +763,7 @@ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause;
748763
*/
749764

750765
/* updated directly by backends and background processes */
751-
extern PGDLLIMPORT PgStat_WalStats PendingWalStats;
766+
extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats;
752767

753768

754769
#endif /* PGSTAT_H */

‎src/tools/pgindent/typedefs.list

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,6 +2053,7 @@ PgStat_Kind
20532053
PgStat_KindInfo
20542054
PgStat_LocalState
20552055
PgStat_PendingDroppedStatsItem
2056+
PgStat_PendingWalStats
20562057
PgStat_SLRUStats
20572058
PgStat_ShmemControl
20582059
PgStat_Snapshot

0 commit comments

Comments
 (0)
0