From 7d00ae64adaed2f1c10c45af79f9eefea86c126a Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot Date: Mon, 1 Sep 2025 10:54:35 +0000 Subject: [PATCH v1] Get rid of pgstat_count_backend_io_op*() functions This commit removes the functions that are incrementing the backend IO stats. Instead, it now copies the IO pending stats to the backend IO pending stats when the pending IO stats are flushed. This behaves the same way as for some relation and database stats. It's done that way to avoid incrementing the "same" counters twice as it produces increased overhead in profiles. Note that per-backend statistics have to be last when we define the PGSTAT_KIND_% values in pgstat_kind.h, as some of their pending stats are populated while other stats kinds are flushing. Reported-by: Andres Freund --- src/backend/utils/activity/pgstat_backend.c | 42 +-------------------- src/backend/utils/activity/pgstat_io.c | 18 +++++---- src/include/pgstat.h | 14 +++---- src/include/utils/pgstat_kind.h | 21 +++++++---- 4 files changed, 31 insertions(+), 64 deletions(-) 59.4% src/backend/utils/activity/ 23.5% src/include/utils/ 17.0% src/include/ diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index 8714a85e2d9..b746b55192e 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -36,8 +36,8 @@ * reported within critical sections so we use static memory in order to avoid * memory allocation. */ -static PgStat_BackendPending PendingBackendStats; -static bool backend_has_iostats = false; +PgStat_BackendPending PendingBackendStats; +bool backend_has_iostats = false; /* * WAL usage counters saved from pgWalUsage at the previous call to @@ -47,44 +47,6 @@ static bool backend_has_iostats = false; */ static WalUsage prevBackendWalUsage; -/* - * Utility routines to report I/O stats for backends, kept here to avoid - * exposing PendingBackendStats to the outside world. - */ -void -pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context, - IOOp io_op, instr_time io_time) -{ - Assert(track_io_timing || track_wal_io_timing); - - if (!pgstat_tracks_backend_bktype(MyBackendType)) - return; - - Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op)); - - INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op], - io_time); - - backend_has_iostats = true; - pgstat_report_fixed = true; -} - -void -pgstat_count_backend_io_op(IOObject io_object, IOContext io_context, - IOOp io_op, uint32 cnt, uint64 bytes) -{ - if (!pgstat_tracks_backend_bktype(MyBackendType)) - return; - - Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op)); - - PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt; - PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes; - - backend_has_iostats = true; - pgstat_report_fixed = true; -} - /* * Returns statistics of a backend by proc number. */ diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 13ae57ed649..0520f4a1d33 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -76,9 +76,6 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, PendingIOStats.counts[io_object][io_context][io_op] += cnt; PendingIOStats.bytes[io_object][io_context][io_op] += bytes; - /* Add the per-backend counts */ - pgstat_count_backend_io_op(io_object, io_context, io_op, cnt, bytes); - have_iostats = true; pgstat_report_fixed = true; } @@ -151,10 +148,6 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op, INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op], io_time); - - /* Add the per-backend count */ - pgstat_count_backend_io_op_time(io_object, io_context, io_op, - io_time); } pgstat_count_io_op(io_object, io_context, io_op, cnt, bytes); @@ -184,6 +177,9 @@ pgstat_flush_io(bool nowait) * * If nowait is true, this function returns true if the lock could not be * acquired. Otherwise, return false. + * + * The stats are copied to the corresponding pending backend stats when + * successfully flushing. */ bool pgstat_io_flush_cb(bool nowait) @@ -227,6 +223,14 @@ pgstat_io_flush_cb(bool nowait) Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)); + /* Do the same for the backend stats */ + if (pgstat_tracks_backend_bktype(MyBackendType)) + { + PendingBackendStats.pending_io = PendingIOStats; + backend_has_iostats = true; + pgstat_report_fixed = true; + } + LWLockRelease(bktype_lock); memset(&PendingIOStats, 0, sizeof(PendingIOStats)); diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 202bd2d5ace..7c2c4a0adc2 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -548,15 +548,6 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void); * Functions in pgstat_backend.c */ -/* used by pgstat_io.c for I/O stats tracked in backends */ -extern void pgstat_count_backend_io_op_time(IOObject io_object, - IOContext io_context, - IOOp io_op, - instr_time io_time); -extern void pgstat_count_backend_io_op(IOObject io_object, - IOContext io_context, - IOOp io_op, uint32 cnt, - uint64 bytes); extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber); extern PgStat_Backend *pgstat_fetch_stat_backend_by_pid(int pid, BackendType *bktype); @@ -800,6 +791,11 @@ extern PGDLLIMPORT bool pgstat_track_counts; extern PGDLLIMPORT int pgstat_track_functions; extern PGDLLIMPORT int pgstat_fetch_consistency; +/* + * Variables in pgstat_backend.c + */ +extern PGDLLIMPORT PgStat_BackendPending PendingBackendStats; +extern PGDLLIMPORT bool backend_has_iostats; /* * Variables in pgstat_bgwriter.c diff --git a/src/include/utils/pgstat_kind.h b/src/include/utils/pgstat_kind.h index eb5f0b3ae6d..4d38f5d922c 100644 --- a/src/include/utils/pgstat_kind.h +++ b/src/include/utils/pgstat_kind.h @@ -29,18 +29,23 @@ #define PGSTAT_KIND_FUNCTION 3 /* per-function statistics */ #define PGSTAT_KIND_REPLSLOT 4 /* per-slot statistics */ #define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */ -#define PGSTAT_KIND_BACKEND 6 /* per-backend statistics */ /* stats for fixed-numbered objects */ -#define PGSTAT_KIND_ARCHIVER 7 -#define PGSTAT_KIND_BGWRITER 8 -#define PGSTAT_KIND_CHECKPOINTER 9 -#define PGSTAT_KIND_IO 10 -#define PGSTAT_KIND_SLRU 11 -#define PGSTAT_KIND_WAL 12 +#define PGSTAT_KIND_ARCHIVER 6 +#define PGSTAT_KIND_BGWRITER 7 +#define PGSTAT_KIND_CHECKPOINTER 8 +#define PGSTAT_KIND_IO 9 +#define PGSTAT_KIND_SLRU 10 +#define PGSTAT_KIND_WAL 11 + +/* + * per-backend statistics has to be last, as some of their pending stats are + * populated while other stats kinds are flushing. + */ +#define PGSTAT_KIND_BACKEND 12 /* per-backend statistics */ #define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE -#define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL +#define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_BACKEND #define PGSTAT_KIND_BUILTIN_SIZE (PGSTAT_KIND_BUILTIN_MAX + 1) /* Custom stats kinds */ -- 2.34.1