From 68c6137702adba99e7699726b1931538a30ccd63 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Thu, 20 Aug 2026 19:08:01 +0000 Subject: [PATCH v7 1/2] Split table stat counters into transactional and non-transactional groups PgStat_TableCounts previously held all per-table counters in a single struct. Split them into two nested structs: PgStat_TableCountsNonTxn for counters that are recorded whether the transaction commits or aborts, such as scans, and PgStat_TableCountsTxn for counters that depend on the transaction outcome, such as tuples inserted. This does not change any behavior; it is groundwork for a future patch that will flush stats that are safe to flush during a transaction. --- src/backend/utils/activity/pgstat_relation.c | 114 +++++++++--------- src/backend/utils/adt/pgstatfuncs.c | 24 ++-- src/include/pgstat.h | 32 +++-- .../test_custom_stats/test_custom_var_stats.c | 11 +- src/tools/pgindent/typedefs.list | 2 + 5 files changed, 104 insertions(+), 79 deletions(-) diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index 17746bf5c54..530e2ae92d2 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -369,7 +369,7 @@ pgstat_report_analyze(Relation rel, deadtuples -= trans->tuples_updated + trans->tuples_deleted; } /* count stuff inserted by already-aborted subxacts, too */ - deadtuples -= rel->pgstat_info->tab.counts.delta_dead_tuples; + deadtuples -= rel->pgstat_info->tab.counts.txn.delta_dead_tuples; /* Since ANALYZE's counts are estimates, we could have underflowed */ livetuples = Max(livetuples, 0); deadtuples = Max(deadtuples, 0); @@ -459,9 +459,9 @@ pgstat_count_heap_update(Relation rel, bool hot, bool newpage) * nontransactional, so just advance them */ if (hot) - pgstat_info->tab.counts.tuples_hot_updated++; + pgstat_info->tab.counts.txn.tuples_hot_updated++; else if (newpage) - pgstat_info->tab.counts.tuples_newpage_updated++; + pgstat_info->tab.counts.txn.tuples_newpage_updated++; } } @@ -519,7 +519,7 @@ pgstat_update_heap_dead_tuples(Relation rel, int delta) Assert(pgstat_info->kind == PGSTAT_KIND_RELATION); - pgstat_info->tab.counts.delta_dead_tuples -= delta; + pgstat_info->tab.counts.txn.delta_dead_tuples -= delta; } } @@ -603,9 +603,9 @@ find_relstat_entry_kind(PgStat_Kind kind, Oid rel_id) */ for (trans = relentry->tab.trans; trans != NULL; trans = trans->upper) { - relstatus->tab.counts.tuples_inserted += trans->tuples_inserted; - relstatus->tab.counts.tuples_updated += trans->tuples_updated; - relstatus->tab.counts.tuples_deleted += trans->tuples_deleted; + relstatus->tab.counts.txn.tuples_inserted += trans->tuples_inserted; + relstatus->tab.counts.txn.tuples_updated += trans->tuples_updated; + relstatus->tab.counts.txn.tuples_deleted += trans->tuples_deleted; } return relstatus; @@ -636,33 +636,33 @@ AtEOXact_PgStat_Relations(PgStat_SubXactStatus *xact_state, bool isCommit) if (!isCommit) restore_truncdrop_counters(trans); /* count attempted actions regardless of commit/abort */ - relstat->tab.counts.tuples_inserted += trans->tuples_inserted; - relstat->tab.counts.tuples_updated += trans->tuples_updated; - relstat->tab.counts.tuples_deleted += trans->tuples_deleted; + relstat->tab.counts.txn.tuples_inserted += trans->tuples_inserted; + relstat->tab.counts.txn.tuples_updated += trans->tuples_updated; + relstat->tab.counts.txn.tuples_deleted += trans->tuples_deleted; if (isCommit) { - relstat->tab.counts.truncdropped = trans->truncdropped; + relstat->tab.counts.txn.truncdropped = trans->truncdropped; if (trans->truncdropped) { /* forget live/dead stats seen by backend thus far */ - relstat->tab.counts.delta_live_tuples = 0; - relstat->tab.counts.delta_dead_tuples = 0; + relstat->tab.counts.txn.delta_live_tuples = 0; + relstat->tab.counts.txn.delta_dead_tuples = 0; } /* insert adds a live tuple, delete removes one */ - relstat->tab.counts.delta_live_tuples += + relstat->tab.counts.txn.delta_live_tuples += trans->tuples_inserted - trans->tuples_deleted; /* update and delete each create a dead tuple */ - relstat->tab.counts.delta_dead_tuples += + relstat->tab.counts.txn.delta_dead_tuples += trans->tuples_updated + trans->tuples_deleted; /* insert, update, delete each count as one change event */ - relstat->tab.counts.changed_tuples += + relstat->tab.counts.txn.changed_tuples += trans->tuples_inserted + trans->tuples_updated + trans->tuples_deleted; } else { /* inserted tuples are dead, deleted tuples are unaffected */ - relstat->tab.counts.delta_dead_tuples += + relstat->tab.counts.txn.delta_dead_tuples += trans->tuples_inserted + trans->tuples_updated; /* an aborted xact generates no changed_tuple events */ } @@ -742,11 +742,11 @@ AtEOSubXact_PgStat_Relations(PgStat_SubXactStatus *xact_state, bool isCommit, in /* first restore values obliterated by truncate/drop */ restore_truncdrop_counters(trans); /* count attempted actions regardless of commit/abort */ - relstat->tab.counts.tuples_inserted += trans->tuples_inserted; - relstat->tab.counts.tuples_updated += trans->tuples_updated; - relstat->tab.counts.tuples_deleted += trans->tuples_deleted; + relstat->tab.counts.txn.tuples_inserted += trans->tuples_inserted; + relstat->tab.counts.txn.tuples_updated += trans->tuples_updated; + relstat->tab.counts.txn.tuples_deleted += trans->tuples_deleted; /* inserted tuples are dead, deleted tuples are unaffected */ - relstat->tab.counts.delta_dead_tuples += + relstat->tab.counts.txn.delta_dead_tuples += trans->tuples_inserted + trans->tuples_updated; relstat->tab.trans = trans->upper; pfree(trans); @@ -826,21 +826,21 @@ pgstat_twophase_postcommit(FullTransactionId fxid, uint16 info, pgstat_info = pgstat_prep_relation_pending(PGSTAT_KIND_RELATION, rec->id, rec->shared); /* Same math as in AtEOXact_PgStat, commit case */ - pgstat_info->tab.counts.tuples_inserted += rec->tuples_inserted; - pgstat_info->tab.counts.tuples_updated += rec->tuples_updated; - pgstat_info->tab.counts.tuples_deleted += rec->tuples_deleted; - pgstat_info->tab.counts.truncdropped = rec->truncdropped; + pgstat_info->tab.counts.txn.tuples_inserted += rec->tuples_inserted; + pgstat_info->tab.counts.txn.tuples_updated += rec->tuples_updated; + pgstat_info->tab.counts.txn.tuples_deleted += rec->tuples_deleted; + pgstat_info->tab.counts.txn.truncdropped = rec->truncdropped; if (rec->truncdropped) { /* forget live/dead stats seen by backend thus far */ - pgstat_info->tab.counts.delta_live_tuples = 0; - pgstat_info->tab.counts.delta_dead_tuples = 0; + pgstat_info->tab.counts.txn.delta_live_tuples = 0; + pgstat_info->tab.counts.txn.delta_dead_tuples = 0; } - pgstat_info->tab.counts.delta_live_tuples += + pgstat_info->tab.counts.txn.delta_live_tuples += rec->tuples_inserted - rec->tuples_deleted; - pgstat_info->tab.counts.delta_dead_tuples += + pgstat_info->tab.counts.txn.delta_dead_tuples += rec->tuples_updated + rec->tuples_deleted; - pgstat_info->tab.counts.changed_tuples += + pgstat_info->tab.counts.txn.changed_tuples += rec->tuples_inserted + rec->tuples_updated + rec->tuples_deleted; } @@ -868,10 +868,10 @@ pgstat_twophase_postabort(FullTransactionId fxid, uint16 info, rec->tuples_updated = rec->updated_pre_truncdrop; rec->tuples_deleted = rec->deleted_pre_truncdrop; } - pgstat_info->tab.counts.tuples_inserted += rec->tuples_inserted; - pgstat_info->tab.counts.tuples_updated += rec->tuples_updated; - pgstat_info->tab.counts.tuples_deleted += rec->tuples_deleted; - pgstat_info->tab.counts.delta_dead_tuples += + pgstat_info->tab.counts.txn.tuples_inserted += rec->tuples_inserted; + pgstat_info->tab.counts.txn.tuples_updated += rec->tuples_updated; + pgstat_info->tab.counts.txn.tuples_deleted += rec->tuples_deleted; + pgstat_info->tab.counts.txn.delta_dead_tuples += rec->tuples_inserted + rec->tuples_updated; } @@ -908,35 +908,35 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) /* add the values to the shared entry. */ tabentry = &shtabstats->stats; - tabentry->numscans += lstats->tab.counts.numscans; - if (lstats->tab.counts.numscans) + tabentry->numscans += lstats->tab.counts.nontxn.numscans; + if (lstats->tab.counts.nontxn.numscans) { TimestampTz t = GetCurrentTransactionStopTimestamp(); if (t > tabentry->lastscan) tabentry->lastscan = t; } - tabentry->tuples_returned += lstats->tab.counts.tuples_returned; - tabentry->tuples_fetched += lstats->tab.counts.tuples_fetched; - tabentry->tuples_inserted += lstats->tab.counts.tuples_inserted; - tabentry->tuples_updated += lstats->tab.counts.tuples_updated; - tabentry->tuples_deleted += lstats->tab.counts.tuples_deleted; - tabentry->tuples_hot_updated += lstats->tab.counts.tuples_hot_updated; - tabentry->tuples_newpage_updated += lstats->tab.counts.tuples_newpage_updated; + tabentry->tuples_returned += lstats->tab.counts.nontxn.tuples_returned; + tabentry->tuples_fetched += lstats->tab.counts.nontxn.tuples_fetched; + tabentry->tuples_inserted += lstats->tab.counts.txn.tuples_inserted; + tabentry->tuples_updated += lstats->tab.counts.txn.tuples_updated; + tabentry->tuples_deleted += lstats->tab.counts.txn.tuples_deleted; + tabentry->tuples_hot_updated += lstats->tab.counts.txn.tuples_hot_updated; + tabentry->tuples_newpage_updated += lstats->tab.counts.txn.tuples_newpage_updated; /* * If table was truncated/dropped, first reset the live/dead counters. */ - if (lstats->tab.counts.truncdropped) + if (lstats->tab.counts.txn.truncdropped) { tabentry->live_tuples = 0; tabentry->dead_tuples = 0; tabentry->ins_since_vacuum = 0; } - tabentry->live_tuples += lstats->tab.counts.delta_live_tuples; - tabentry->dead_tuples += lstats->tab.counts.delta_dead_tuples; - tabentry->mod_since_analyze += lstats->tab.counts.changed_tuples; + tabentry->live_tuples += lstats->tab.counts.txn.delta_live_tuples; + tabentry->dead_tuples += lstats->tab.counts.txn.delta_dead_tuples; + tabentry->mod_since_analyze += lstats->tab.counts.txn.changed_tuples; /* * Using tuples_inserted to update ins_since_vacuum does mean that we'll @@ -945,10 +945,10 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) * triggering for inserts more often than they maybe should, which is * probably not going to be common enough to be too concerned about here. */ - tabentry->ins_since_vacuum += lstats->tab.counts.tuples_inserted; + tabentry->ins_since_vacuum += lstats->tab.counts.txn.tuples_inserted; - tabentry->blocks_fetched += lstats->tab.counts.blocks_fetched; - tabentry->blocks_hit += lstats->tab.counts.blocks_hit; + tabentry->blocks_fetched += lstats->tab.counts.nontxn.blocks_fetched; + tabentry->blocks_hit += lstats->tab.counts.nontxn.blocks_hit; /* Clamp live_tuples in case of negative delta_live_tuples */ tabentry->live_tuples = Max(tabentry->live_tuples, 0); @@ -959,13 +959,13 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) /* The entry was successfully flushed, add the same to database stats */ dbentry = pgstat_prep_database_pending(dboid); - dbentry->tuples_returned += lstats->tab.counts.tuples_returned; - dbentry->tuples_fetched += lstats->tab.counts.tuples_fetched; - dbentry->tuples_inserted += lstats->tab.counts.tuples_inserted; - dbentry->tuples_updated += lstats->tab.counts.tuples_updated; - dbentry->tuples_deleted += lstats->tab.counts.tuples_deleted; - dbentry->blocks_fetched += lstats->tab.counts.blocks_fetched; - dbentry->blocks_hit += lstats->tab.counts.blocks_hit; + dbentry->tuples_returned += lstats->tab.counts.nontxn.tuples_returned; + dbentry->tuples_fetched += lstats->tab.counts.nontxn.tuples_fetched; + dbentry->tuples_inserted += lstats->tab.counts.txn.tuples_inserted; + dbentry->tuples_updated += lstats->tab.counts.txn.tuples_updated; + dbentry->tuples_deleted += lstats->tab.counts.txn.tuples_deleted; + dbentry->blocks_fetched += lstats->tab.counts.nontxn.blocks_fetched; + dbentry->blocks_hit += lstats->tab.counts.nontxn.blocks_hit; return true; } diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 0d47d745c18..5c3b1d51091 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1922,7 +1922,7 @@ pg_stat_get_slru(PG_FUNCTION_ARGS) return (Datum) 0; } -#define PG_STAT_GET_XACT_RELENTRY_INT64(stat) \ +#define PG_STAT_GET_XACT_RELENTRY_INT64(group, stat) \ Datum \ CppConcat(pg_stat_get_xact_,stat)(PG_FUNCTION_ARGS) \ { \ @@ -1934,40 +1934,40 @@ CppConcat(pg_stat_get_xact_,stat)(PG_FUNCTION_ARGS) \ relid)) == NULL) \ result = 0; \ else \ - result = (int64) (tabentry->tab.counts.stat); \ + result = (int64) (tabentry->tab.counts.group.stat); \ \ PG_RETURN_INT64(result); \ } /* pg_stat_get_xact_numscans */ -PG_STAT_GET_XACT_RELENTRY_INT64(numscans) +PG_STAT_GET_XACT_RELENTRY_INT64(nontxn, numscans) /* pg_stat_get_xact_tuples_returned */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_returned) +PG_STAT_GET_XACT_RELENTRY_INT64(nontxn, tuples_returned) /* pg_stat_get_xact_tuples_fetched */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_fetched) +PG_STAT_GET_XACT_RELENTRY_INT64(nontxn, tuples_fetched) /* pg_stat_get_xact_tuples_hot_updated */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_hot_updated) +PG_STAT_GET_XACT_RELENTRY_INT64(txn, tuples_hot_updated) /* pg_stat_get_xact_tuples_newpage_updated */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_newpage_updated) +PG_STAT_GET_XACT_RELENTRY_INT64(txn, tuples_newpage_updated) /* pg_stat_get_xact_blocks_fetched */ -PG_STAT_GET_XACT_RELENTRY_INT64(blocks_fetched) +PG_STAT_GET_XACT_RELENTRY_INT64(nontxn, blocks_fetched) /* pg_stat_get_xact_blocks_hit */ -PG_STAT_GET_XACT_RELENTRY_INT64(blocks_hit) +PG_STAT_GET_XACT_RELENTRY_INT64(nontxn, blocks_hit) /* pg_stat_get_xact_tuples_inserted */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_inserted) +PG_STAT_GET_XACT_RELENTRY_INT64(txn, tuples_inserted) /* pg_stat_get_xact_tuples_updated */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_updated) +PG_STAT_GET_XACT_RELENTRY_INT64(txn, tuples_updated) /* pg_stat_get_xact_tuples_deleted */ -PG_STAT_GET_XACT_RELENTRY_INT64(tuples_deleted) +PG_STAT_GET_XACT_RELENTRY_INT64(txn, tuples_deleted) /* * Accessor macro for in-transaction index stats. diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 204782fd630..b6b262c7064 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -121,7 +121,12 @@ typedef struct PgStat_BackendSubEntry /* ---------- * PgStat_TableCounts The actual per-table counts kept by a backend * - * This struct should contain only actual event counters, because we make use + * The counters are split into two structs. PgStat_TableCountsNonTxn holds + * counters that are recorded whether the transaction commits or aborts, while + * PgStat_TableCountsTxn holds counters whose effect depends on the transaction + * outcome. Both are combined in PgStat_TableCounts. + * + * These structs should contain only actual event counters, because we make use * of pg_memory_is_all_zeros() to detect whether there are any stats updates * to apply. * @@ -138,13 +143,19 @@ typedef struct PgStat_BackendSubEntry * Note that delta_live_tuples and delta_dead_tuples can be negative! * ---------- */ -typedef struct PgStat_TableCounts +typedef struct PgStat_TableCountsNonTxn { PgStat_Counter numscans; PgStat_Counter tuples_returned; PgStat_Counter tuples_fetched; + PgStat_Counter blocks_fetched; + PgStat_Counter blocks_hit; +} PgStat_TableCountsNonTxn; + +typedef struct PgStat_TableCountsTxn +{ PgStat_Counter tuples_inserted; PgStat_Counter tuples_updated; PgStat_Counter tuples_deleted; @@ -155,9 +166,12 @@ typedef struct PgStat_TableCounts PgStat_Counter delta_live_tuples; PgStat_Counter delta_dead_tuples; PgStat_Counter changed_tuples; +} PgStat_TableCountsTxn; - PgStat_Counter blocks_fetched; - PgStat_Counter blocks_hit; +typedef struct PgStat_TableCounts +{ + PgStat_TableCountsNonTxn nontxn; + PgStat_TableCountsTxn txn; } PgStat_TableCounts; /* ---------- @@ -779,7 +793,7 @@ extern void pgstat_report_analyze(Relation rel, if (pgstat_should_count_relation(rel)) \ { \ Assert((rel)->pgstat_info->kind == PGSTAT_KIND_RELATION); \ - (rel)->pgstat_info->tab.counts.numscans++; \ + (rel)->pgstat_info->tab.counts.nontxn.numscans++; \ } \ } while (0) #define pgstat_count_heap_getnext(rel) \ @@ -787,7 +801,7 @@ extern void pgstat_report_analyze(Relation rel, if (pgstat_should_count_relation(rel)) \ { \ Assert((rel)->pgstat_info->kind == PGSTAT_KIND_RELATION); \ - (rel)->pgstat_info->tab.counts.tuples_returned++; \ + (rel)->pgstat_info->tab.counts.nontxn.tuples_returned++; \ } \ } while (0) #define pgstat_count_heap_fetch(rel) \ @@ -797,7 +811,7 @@ extern void pgstat_report_analyze(Relation rel, if ((rel)->pgstat_info->kind == PGSTAT_KIND_INDEX) \ (rel)->pgstat_info->idx.tuples_fetched++; \ else \ - (rel)->pgstat_info->tab.counts.tuples_fetched++; \ + (rel)->pgstat_info->tab.counts.nontxn.tuples_fetched++; \ } \ } while (0) #define pgstat_count_index_scan(rel) \ @@ -823,7 +837,7 @@ extern void pgstat_report_analyze(Relation rel, if ((rel)->pgstat_info->kind == PGSTAT_KIND_INDEX) \ (rel)->pgstat_info->idx.blocks_fetched++; \ else \ - (rel)->pgstat_info->tab.counts.blocks_fetched++; \ + (rel)->pgstat_info->tab.counts.nontxn.blocks_fetched++; \ } \ } while (0) #define pgstat_count_buffer_hit(rel) \ @@ -833,7 +847,7 @@ extern void pgstat_report_analyze(Relation rel, if ((rel)->pgstat_info->kind == PGSTAT_KIND_INDEX) \ (rel)->pgstat_info->idx.blocks_hit++; \ else \ - (rel)->pgstat_info->tab.counts.blocks_hit++; \ + (rel)->pgstat_info->tab.counts.nontxn.blocks_hit++; \ } \ } while (0) diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c index a39ada0b67c..bf4c8fa69c3 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c @@ -56,10 +56,19 @@ PG_MODULE_MAGIC_EXT( *-------------------------------------------------------------------------- */ -/* Backend-local pending statistics before flush to shared memory */ +/* + * Backend-local pending statistics before flush to shared memory. + * + * numcalls is non-transactional and is flushed on any flush, including an + * in-transaction one. numcalls_txn is transactional and becomes visible in + * shared memory only at a transaction boundary; this demonstrates how a custom + * kind can defer transaction-dependent counters (see the flush callback). + */ typedef struct PgStat_StatCustomVarEntry { PgStat_Counter numcalls; /* times statistic was incremented */ + PgStat_Counter numcalls_txn; /* likewise, but flushed only at a + * transaction boundary */ } PgStat_StatCustomVarEntry; /* Shared memory statistics entry visible to all backends */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 751daacede1..c6c80da2b38 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2371,6 +2371,8 @@ PgStat_StatTabEntry PgStat_StatsFileOp PgStat_SubXactStatus PgStat_TableCounts +PgStat_TableCountsNonTxn +PgStat_TableCountsTxn PgStat_TableXactStatus PgStat_WalCounters PgStat_WalStats -- 2.47.3