From f8a7e590b9bfb240bf807d32543192c9486d1d21 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz Date: Thu, 2 Feb 2023 15:06:48 +0300 Subject: [PATCH v3] Refactor instr_time calculations --- src/backend/access/transam/xlog.c | 6 ++--- src/backend/storage/file/buffile.c | 6 ++--- src/backend/utils/activity/pgstat_wal.c | 31 +++++++++++++------------ src/include/pgstat.h | 18 +++++++++++++- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f9f0f6db8d..3c35dc1ca2 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2206,8 +2206,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible) instr_time duration; INSTR_TIME_SET_CURRENT(duration); - INSTR_TIME_SUBTRACT(duration, start); - PendingWalStats.wal_write_time += INSTR_TIME_GET_MICROSEC(duration); + INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_write_time, duration, start); } PendingWalStats.wal_write++; @@ -8201,8 +8200,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli) instr_time duration; INSTR_TIME_SET_CURRENT(duration); - INSTR_TIME_SUBTRACT(duration, start); - PendingWalStats.wal_sync_time += INSTR_TIME_GET_MICROSEC(duration); + INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, duration, start); } PendingWalStats.wal_sync++; diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index 0a51624df3..e55f86b675 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -469,8 +469,7 @@ BufFileLoadBuffer(BufFile *file) if (track_io_timing) { INSTR_TIME_SET_CURRENT(io_time); - INSTR_TIME_SUBTRACT(io_time, io_start); - INSTR_TIME_ADD(pgBufferUsage.temp_blk_read_time, io_time); + INSTR_TIME_ACCUM_DIFF(pgBufferUsage.temp_blk_read_time, io_time, io_start); } /* we choose not to advance curOffset here */ @@ -544,8 +543,7 @@ BufFileDumpBuffer(BufFile *file) if (track_io_timing) { INSTR_TIME_SET_CURRENT(io_time); - INSTR_TIME_SUBTRACT(io_time, io_start); - INSTR_TIME_ADD(pgBufferUsage.temp_blk_write_time, io_time); + INSTR_TIME_ACCUM_DIFF(pgBufferUsage.temp_blk_write_time, io_time, io_start); } file->curOffset += bytestowrite; diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c index e8598b2f4e..0979d1c269 100644 --- a/src/backend/utils/activity/pgstat_wal.c +++ b/src/backend/utils/activity/pgstat_wal.c @@ -21,7 +21,7 @@ #include "executor/instrument.h" -PgStat_WalStats PendingWalStats = {0}; +PgStat_PendingWalStats PendingWalStats = {0}; /* * WAL usage counters saved from pgWALUsage at the previous call to @@ -70,7 +70,7 @@ bool pgstat_flush_wal(bool nowait) { PgStatShared_Wal *stats_shmem = &pgStatLocal.shmem->wal; - WalUsage diff = {0}; + WalUsage wal_usage_diff = {0}; Assert(IsUnderPostmaster || !IsPostmasterEnvironment); Assert(pgStatLocal.shmem != NULL && @@ -88,25 +88,26 @@ pgstat_flush_wal(bool nowait) * Calculate how much WAL usage counters were increased by subtracting the * previous counters from the current ones. */ - WalUsageAccumDiff(&diff, &pgWalUsage, &prevWalUsage); - PendingWalStats.wal_records = diff.wal_records; - PendingWalStats.wal_fpi = diff.wal_fpi; - PendingWalStats.wal_bytes = diff.wal_bytes; + WalUsageAccumDiff(&wal_usage_diff, &pgWalUsage, &prevWalUsage); if (!nowait) LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); else if (!LWLockConditionalAcquire(&stats_shmem->lock, LW_EXCLUSIVE)) return true; -#define WALSTAT_ACC(fld) stats_shmem->stats.fld += PendingWalStats.fld - WALSTAT_ACC(wal_records); - WALSTAT_ACC(wal_fpi); - WALSTAT_ACC(wal_bytes); - WALSTAT_ACC(wal_buffers_full); - WALSTAT_ACC(wal_write); - WALSTAT_ACC(wal_sync); - WALSTAT_ACC(wal_write_time); - WALSTAT_ACC(wal_sync_time); +#define WALSTAT_ACC(fld, var_to_add) \ + (stats_shmem->stats.fld += var_to_add.fld) +#define WALLSTAT_ACC_INSTR_TIME_TYPE(fld) \ + (stats_shmem->stats.fld += INSTR_TIME_GET_MICROSEC(PendingWalStats.fld)) + WALSTAT_ACC(wal_records, wal_usage_diff); + WALSTAT_ACC(wal_fpi, wal_usage_diff); + WALSTAT_ACC(wal_bytes, wal_usage_diff); + WALSTAT_ACC(wal_buffers_full, PendingWalStats); + WALSTAT_ACC(wal_write, PendingWalStats); + WALSTAT_ACC(wal_sync, PendingWalStats); + WALLSTAT_ACC_INSTR_TIME_TYPE(wal_write_time); + WALLSTAT_ACC_INSTR_TIME_TYPE(wal_sync_time); +#undef WALLSTAT_ACC_INSTR_TIME_TYPE #undef WALSTAT_ACC LWLockRelease(&stats_shmem->lock); diff --git a/src/include/pgstat.h b/src/include/pgstat.h index db9675884f..4be6fdeaf5 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -445,6 +445,22 @@ typedef struct PgStat_WalStats TimestampTz stat_reset_timestamp; } PgStat_WalStats; +/* + * Created for accumulating wal_write_time and wal_sync_time as a instr_time + * but instr_time can't be used as a type where it ends up on-disk + * because its units may change. PgStat_WalStats type is used for + * in-memory/on-disk data. So, PgStat_PendingWalStats is created for + * accumulating intervals as a instr_time. + */ +typedef struct PgStat_PendingWalStats +{ + PgStat_Counter wal_buffers_full; + PgStat_Counter wal_write; + PgStat_Counter wal_sync; + instr_time wal_write_time; + instr_time wal_sync_time; +} PgStat_PendingWalStats; + /* * Functions in pgstat.c @@ -758,7 +774,7 @@ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause; */ /* updated directly by backends and background processes */ -extern PGDLLIMPORT PgStat_WalStats PendingWalStats; +extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats; #endif /* PGSTAT_H */ -- 2.25.1