Hi, On Tue, Aug 26, 2025 at 06:38:41AM +0000, Bertrand Drouvot wrote: > Hi, > > On Mon, Aug 25, 2025 at 08:28:04PM -0400, Andres Freund wrote: > > I don't like that this basically doubles the overhead of keeping stats by > > tracking everythign twice. The proper solution is to do that not in the hot > > path (i.e. in scans), but when summarizing stats to be flushed to the shared > > stats. > > I do agree, like when the relations stats are flushed then we do update the > database ones too. I'll use the same approach in the next revision.
Something along the lines like in the attached (I'm just providing 0001 here, will do the others if we agree on the proposal)? Remark: We could avoid the new branch in pgstat_relation_flush_cb() if we split tables and indexes stats, but I don't think that should be a blocker for per backend relations stats. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 5be3ea4753db28cf0c15da65bd6263e0b9f964a4 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Mon, 4 Aug 2025 08:14:02 +0000 Subject: [PATCH v2 1/2] Adding per backend relation statistics tracking This commit introduces per backend relation stats tracking and adds a new PgStat_BackendRelPending struct to store the pending statistics. To begin with, this commit adds a new counter numscans to record the number of sequential scans initiated on tables. This commit relies on the existing per backend statistics machinery that has been added in 9aea73fc61d. --- src/backend/utils/activity/pgstat_backend.c | 47 +++++++++++++++++++- src/backend/utils/activity/pgstat_relation.c | 7 +++ src/include/pgstat.h | 14 ++++++ src/include/utils/pgstat_internal.h | 3 +- src/tools/pgindent/typedefs.list | 1 + 5 files changed, 70 insertions(+), 2 deletions(-) 75.4% src/backend/utils/activity/ 7.9% src/include/utils/ 15.3% src/include/ diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index 8714a85e2d9..0644e999e93 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -36,7 +36,7 @@ * reported within critical sections so we use static memory in order to avoid * memory allocation. */ -static PgStat_BackendPending PendingBackendStats; +PgStat_BackendPending PendingBackendStats; static bool backend_has_iostats = false; /* @@ -47,6 +47,11 @@ static bool backend_has_iostats = false; */ static WalUsage prevBackendWalUsage; +/* + * For backend relations's related statistics. + */ +bool backend_has_relstats = false; + /* * Utility routines to report I/O stats for backends, kept here to avoid * exposing PendingBackendStats to the outside world. @@ -259,6 +264,39 @@ pgstat_flush_backend_entry_wal(PgStat_EntryRef *entry_ref) prevBackendWalUsage = pgWalUsage; } +/* + * Flush out locally pending backend relations's related statistics. Locking is + * managed by the caller. + */ +static void +pgstat_flush_backend_entry_rel(PgStat_EntryRef *entry_ref) +{ + PgStatShared_Backend *shbackendent; + + /* + * This function can be called even if nothing at all has happened for + * relations's related statistics. In this case, avoid unnecessarily + * modifying the stats entry. + */ + if (!backend_has_relstats) + return; + + shbackendent = (PgStatShared_Backend *) entry_ref->shared_stats; + +#define BACKENDREL_ACC(stat) \ + (shbackendent->stats.stat += PendingBackendStats.pending_backendrel.stat) + + BACKENDREL_ACC(heap_scan); +#undef BACKENDREL_ACC + + /* + * Clear out the statistics buffer, so it can be re-used. + */ + MemSet(&PendingBackendStats.pending_backendrel, 0, sizeof(PgStat_BackendRelPending)); + + backend_has_relstats = false; +} + /* * Flush out locally pending backend statistics * @@ -283,6 +321,10 @@ pgstat_flush_backend(bool nowait, bits32 flags) pgstat_backend_wal_have_pending()) has_pending_data = true; + /* Some relations related data pending? */ + if ((flags & PGSTAT_BACKEND_FLUSH_REL) && backend_has_relstats) + has_pending_data = true; + if (!has_pending_data) return false; @@ -298,6 +340,9 @@ pgstat_flush_backend(bool nowait, bits32 flags) if (flags & PGSTAT_BACKEND_FLUSH_WAL) pgstat_flush_backend_entry_wal(entry_ref); + if (flags & PGSTAT_BACKEND_FLUSH_REL) + pgstat_flush_backend_entry_rel(entry_ref); + pgstat_unlock_entry(entry_ref); return false; diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index 69df741cbf6..f2318bddb41 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -898,6 +898,13 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) dbentry->blocks_fetched += lstats->counts.blocks_fetched; dbentry->blocks_hit += lstats->counts.blocks_hit; + /* Do the same for backend stats */ + if (lstats->relation && lstats->relation->rd_rel->relkind == RELKIND_RELATION) + PendingBackendStats.pending_backendrel.heap_scan += lstats->counts.numscans; + + backend_has_relstats = true; + pgstat_report_fixed = true; + return true; } diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 202bd2d5ace..fb20cd96bd6 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -490,8 +490,14 @@ typedef struct PgStat_Backend TimestampTz stat_reset_timestamp; PgStat_BktypeIO io_stats; PgStat_WalCounters wal_counters; + PgStat_Counter heap_scan; } PgStat_Backend; +typedef struct PgStat_BackendRelPending +{ + PgStat_Counter heap_scan; +} PgStat_BackendRelPending; + /* --------- * PgStat_BackendPending Non-flushed backend stats. * --------- @@ -502,8 +508,16 @@ typedef struct PgStat_BackendPending * Backend statistics store the same amount of IO data as PGSTAT_KIND_IO. */ PgStat_PendingIO pending_io; + + /* + * Backend statistics related to relations. + */ + PgStat_BackendRelPending pending_backendrel; } PgStat_BackendPending; +extern PgStat_BackendPending PendingBackendStats; +extern bool backend_has_relstats; + /* * Functions in pgstat.c */ diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 6cf00008f63..286249c0f3a 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -616,7 +616,8 @@ extern void pgstat_archiver_snapshot_cb(void); /* flags for pgstat_flush_backend() */ #define PGSTAT_BACKEND_FLUSH_IO (1 << 0) /* Flush I/O statistics */ #define PGSTAT_BACKEND_FLUSH_WAL (1 << 1) /* Flush WAL statistics */ -#define PGSTAT_BACKEND_FLUSH_ALL (PGSTAT_BACKEND_FLUSH_IO | PGSTAT_BACKEND_FLUSH_WAL) +#define PGSTAT_BACKEND_FLUSH_REL (1 << 2) /* Flush relations related statistics */ +#define PGSTAT_BACKEND_FLUSH_ALL (PGSTAT_BACKEND_FLUSH_IO | PGSTAT_BACKEND_FLUSH_WAL | PGSTAT_BACKEND_FLUSH_REL) extern bool pgstat_flush_backend(bool nowait, bits32 flags); extern bool pgstat_backend_flush_cb(bool nowait); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a13e8162890..e3f4c71466b 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2227,6 +2227,7 @@ PgStatShared_Wal PgStat_ArchiverStats PgStat_Backend PgStat_BackendPending +PgStat_BackendRelPending PgStat_BackendSubEntry PgStat_BgWriterStats PgStat_BktypeIO -- 2.34.1
