From 8fca00cec38f2459baea3f11a06df79b94572186 Mon Sep 17 00:00:00 2001 From: alterego655 <824662526@qq.com> Date: Tue, 12 May 2026 11:55:44 +0800 Subject: [PATCH v2] Fix pg_stat_xact_* views leaking across xact boundaries pgStatPending entries can persist across transaction boundaries within a backend. The pg_stat_xact_* view accessors read raw accumulated counters from these entries, so they can report activity from prior transactions instead of just the current one. Fix this by recording a baseline snapshot of pending relation and function counters at each top-level transaction boundary. pgstat_set_pending_baselines() walks pgStatPending at commit, abort, and PREPARE TRANSACTION. View accessors subtract the saved baseline to produce the current transaction's delta. Entries first created in the current transaction keep a zero baseline, so their delta is unchanged. For function stats, add PgStat_FunctionPending to pair PgStat_FunctionCounts with a baseline copy. find_funcstat_entry() now returns a palloc'd delta, or NULL when the function was not called in the current transaction. Add the new typedef to pgindent's typedef list. Add a test_misc TAP test covering counter isolation across consecutive transactions in a single simple-query message. --- src/backend/utils/activity/pgstat.c | 42 +++- src/backend/utils/activity/pgstat_function.c | 41 +++- src/backend/utils/activity/pgstat_relation.c | 20 ++ src/backend/utils/activity/pgstat_xact.c | 15 ++ src/include/pgstat.h | 16 ++ src/include/utils/pgstat_internal.h | 1 + src/test/modules/test_misc/meson.build | 1 + .../modules/test_misc/t/013_pgstat_xact.pl | 211 ++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 9 files changed, 338 insertions(+), 10 deletions(-) create mode 100644 src/test/modules/test_misc/t/013_pgstat_xact.pl diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index b67da88c7dc..a414a488320 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -326,7 +326,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_size = sizeof(PgStatShared_Function), .shared_data_off = offsetof(PgStatShared_Function, stats), .shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats), - .pending_size = sizeof(PgStat_FunctionCounts), + .pending_size = sizeof(PgStat_FunctionPending), .flush_pending_cb = pgstat_function_flush_cb, .reset_timestamp_cb = pgstat_function_reset_timestamp_cb, @@ -1357,6 +1357,46 @@ pgstat_fetch_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid) return entry_ref; } +/* + * Set xact baselines for all pending relation and function entries. + * + * Called at top-level transaction commit, abort, and successful PREPARE to + * record the current counter state as the baseline. The next transaction on + * this backend will compute xact-scoped deltas by subtracting this baseline. + * + * This sweeps all unflushed pending entries, not just those touched in the + * current transaction, because nontransactional counters (scans, buffer hits, + * etc.) accumulate in the pending entry regardless of whether a + * PgStat_TableXactStatus was created. + */ +void +pgstat_set_pending_baselines(void) +{ + dlist_iter iter; + + dlist_foreach(iter, &pgStatPending) + { + PgStat_EntryRef *entry_ref = + dlist_container(PgStat_EntryRef, pending_node, iter.cur); + PgStat_Kind kind = entry_ref->shared_entry->key.kind; + + if (kind == PGSTAT_KIND_RELATION) + { + PgStat_TableStatus *tabstat = + (PgStat_TableStatus *) entry_ref->pending; + + tabstat->xact_baseline = tabstat->counts; + } + else if (kind == PGSTAT_KIND_FUNCTION) + { + PgStat_FunctionPending *fpending = + (PgStat_FunctionPending *) entry_ref->pending; + + fpending->xact_baseline = fpending->counts; + } + } +} + void pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref) { diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c index d47d05e3d92..04bbe2e7cb6 100644 --- a/src/backend/utils/activity/pgstat_function.c +++ b/src/backend/utils/activity/pgstat_function.c @@ -119,7 +119,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo, } } - pending = entry_ref->pending; + pending = &((PgStat_FunctionPending *) entry_ref->pending)->counts; fcu->fs = pending; @@ -192,10 +192,12 @@ pgstat_end_function_usage(PgStat_FunctionCallUsage *fcu, bool finalize) bool pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) { + PgStat_FunctionPending *fpending; PgStat_FunctionCounts *localent; PgStatShared_Function *shfuncent; - localent = (PgStat_FunctionCounts *) entry_ref->pending; + fpending = (PgStat_FunctionPending *) entry_ref->pending; + localent = &fpending->counts; shfuncent = (PgStatShared_Function *) entry_ref->shared_stats; /* localent always has non-zero content */ @@ -223,18 +225,39 @@ pgstat_function_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts) /* * find any existing PgStat_FunctionCounts entry for specified function * - * If no entry, return NULL, don't create a new one + * Returns a palloc'd delta struct showing only the current transaction's + * contribution, or NULL if the function was not called in this transaction. */ PgStat_FunctionCounts * find_funcstat_entry(Oid func_id) { PgStat_EntryRef *entry_ref; - - entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId, func_id); - - if (entry_ref) - return entry_ref->pending; - return NULL; + PgStat_FunctionPending *fpending; + PgStat_FunctionCounts *result; + PgStat_Counter delta_calls; + + entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_FUNCTION, + MyDatabaseId, func_id); + if (!entry_ref) + return NULL; + + fpending = (PgStat_FunctionPending *) entry_ref->pending; + delta_calls = fpending->counts.numcalls - fpending->xact_baseline.numcalls; + if (delta_calls == 0) + return NULL; + + result = palloc(sizeof(PgStat_FunctionCounts)); + result->numcalls = delta_calls; + INSTR_TIME_SET_ZERO(result->total_time); + INSTR_TIME_ACCUM_DIFF(result->total_time, + fpending->counts.total_time, + fpending->xact_baseline.total_time); + INSTR_TIME_SET_ZERO(result->self_time); + INSTR_TIME_ACCUM_DIFF(result->self_time, + fpending->counts.self_time, + fpending->xact_baseline.self_time); + + return result; } /* diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index b2ca28f83ba..0de69ef8b1d 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -505,6 +505,7 @@ find_tabstat_entry(Oid rel_id) PgStat_TableXactStatus *trans; PgStat_TableStatus *tabentry = NULL; PgStat_TableStatus *tablestatus = NULL; + PgStat_TableCounts *baseline; entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION, MyDatabaseId, rel_id); if (!entry_ref) @@ -525,6 +526,25 @@ find_tabstat_entry(Oid rel_id) */ tablestatus->trans = NULL; + /* + * Subtract the xact baseline to show only the current top-level + * transaction's activity. Only subtract counters exposed through + * pg_stat_xact_* views. The remaining PgStat_TableCounts fields are + * cumulative relation-maintenance state, not xact-view output. + */ + baseline = &tabentry->xact_baseline; + tablestatus->counts.numscans -= baseline->numscans; + tablestatus->counts.tuples_returned -= baseline->tuples_returned; + tablestatus->counts.tuples_fetched -= baseline->tuples_fetched; + tablestatus->counts.tuples_inserted -= baseline->tuples_inserted; + tablestatus->counts.tuples_updated -= baseline->tuples_updated; + tablestatus->counts.tuples_deleted -= baseline->tuples_deleted; + tablestatus->counts.tuples_hot_updated -= baseline->tuples_hot_updated; + tablestatus->counts.tuples_newpage_updated -= + baseline->tuples_newpage_updated; + tablestatus->counts.blocks_fetched -= baseline->blocks_fetched; + tablestatus->counts.blocks_hit -= baseline->blocks_hit; + /* * Live subtransaction counts are not included yet. This is not a hot * code path so reconcile tuples_inserted, tuples_updated and diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c index 5e2d69e6297..ad8ae891113 100644 --- a/src/backend/utils/activity/pgstat_xact.c +++ b/src/backend/utils/activity/pgstat_xact.c @@ -55,6 +55,14 @@ AtEOXact_PgStat(bool isCommit, bool parallel) } pgStatXactStack = NULL; + /* + * Record current counter values as the baseline for the next transaction. + * This must be after AtEOXact_PgStat_Relations(), which folds + * transactional counts into ->counts, and AtEOXact_PgStat_DroppedStats(), + * which removes dropped entries from pgStatPending. + */ + pgstat_set_pending_baselines(); + /* Make sure any stats snapshot is thrown away */ pgstat_clear_snapshot(); } @@ -226,6 +234,13 @@ PostPrepare_PgStat(void) } pgStatXactStack = NULL; + /* + * Record baselines, same as at commit/abort. AtEOXact_PgStat() is not + * called during PREPARE, so without this, counters from the prepared + * transaction would leak into later transactions. + */ + pgstat_set_pending_baselines(); + /* Make sure any stats snapshot is thrown away */ pgstat_clear_snapshot(); } diff --git a/src/include/pgstat.h b/src/include/pgstat.h index dfa2e837638..dec7fcec8e0 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -90,6 +90,21 @@ typedef struct PgStat_FunctionCounts instr_time self_time; } PgStat_FunctionCounts; +/* ---------- + * PgStat_FunctionPending Backend-local pending state for function stats + * + * Wraps the accumulated counts with a per-transaction baseline so that + * pg_stat_xact_user_functions can report only the current transaction's + * contribution. The baseline is set at each top-level transaction boundary + * by pgstat_set_pending_baselines(). + * ---------- + */ +typedef struct PgStat_FunctionPending +{ + PgStat_FunctionCounts counts; /* accumulated counts */ + PgStat_FunctionCounts xact_baseline; /* snapshot at xact boundary */ +} PgStat_FunctionPending; + /* * Working state needed to accumulate per-function-call timing statistics. */ @@ -183,6 +198,7 @@ typedef struct PgStat_TableStatus bool shared; /* is it a shared catalog? */ struct PgStat_TableXactStatus *trans; /* lowest subxact's counts */ PgStat_TableCounts counts; /* event counts to be sent */ + PgStat_TableCounts xact_baseline; /* snapshot at xact boundary */ Relation relation; /* rel that is using this entry */ } PgStat_TableStatus; diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index fe463faaf63..f8bb8a4f230 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -684,6 +684,7 @@ extern PgStat_EntryRef *pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, bool *created_entry); extern PgStat_EntryRef *pgstat_fetch_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid); +extern void pgstat_set_pending_baselines(void); extern void *pgstat_fetch_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *may_free); diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 356d8454b39..3bc56034fc0 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -21,6 +21,7 @@ tests += { 't/010_index_concurrently_upsert.pl', 't/011_lock_stats.pl', 't/012_ddlutils.pl', + 't/013_pgstat_xact.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/013_pgstat_xact.pl b/src/test/modules/test_misc/t/013_pgstat_xact.pl new file mode 100644 index 00000000000..03ad8ba2101 --- /dev/null +++ b/src/test/modules/test_misc/t/013_pgstat_xact.pl @@ -0,0 +1,211 @@ +# Copyright (c) 2021-2026, PostgreSQL Global Development Group + +# Test that pg_stat_xact_* views report only current-transaction activity, +# not accumulated pending stats from prior transactions. +# +# Use psql -c so all statements in a command are sent as one simple-query +# message. That preserves pgStatPending entries across consecutive top-level +# transactions by avoiding a ReadyForQuery boundary, where PostgresMain() may +# call pgstat_report_stat(). + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; +$node->append_conf('postgresql.conf', "track_functions = 'all'"); +$node->append_conf('postgresql.conf', "max_prepared_transactions = 5"); +$node->start; + +my $dbname = 'postgres'; + +$node->safe_psql( + $dbname, q{ + CREATE TABLE test_xact_stats (x int); + CREATE FUNCTION test_xact_stats_func() RETURNS void + LANGUAGE plpgsql AS $$ BEGIN NULL; END; $$; +}); + +sub psql_c_sequence_like +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my ($node, $commands, $expected_stdout, $test_name) = @_; + my ($stdout, $stderr); + my @cmd = ( + $node->installed_command('psql'), + '--no-psqlrc', + '--no-align', + '--tuples-only', + '--quiet', + '--set' => 'ON_ERROR_STOP=1', + '-d' => $node->connstr($dbname)); + + foreach my $command (@{$commands}) + { + push @cmd, '-c' => $command; + } + + my $result = IPC::Run::run \@cmd, '>', \$stdout, '2>', \$stderr; + + is($result, 1, "$test_name: exit code 0"); + is($stderr, '', "$test_name: no stderr"); + like($stdout, $expected_stdout, "$test_name: matches"); +} + +sub psql_c_like +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my ($node, $sql, $expected_stdout, $test_name) = @_; + + psql_c_sequence_like($node, [$sql], $expected_stdout, $test_name); +} + +psql_c_like( + $node, + q{BEGIN; + INSERT INTO test_xact_stats VALUES (1); + COMMIT; + BEGIN; + INSERT INTO test_xact_stats VALUES (2); + SELECT 'n_tup_ins:' || n_tup_ins FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;}, + qr/^n_tup_ins:1$/m, + "table n_tup_ins shows only current transaction's inserts"); + +psql_c_like( + $node, + q{BEGIN; + SELECT count(*) FROM test_xact_stats; + COMMIT; + BEGIN; + SELECT count(*) FROM test_xact_stats; + SELECT 'seq_scan:' || seq_scan FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;}, + qr/^seq_scan:1$/m, + "table seq_scan shows only current transaction's scans"); + +psql_c_like( + $node, + q{BEGIN; + SELECT test_xact_stats_func(); + SELECT test_xact_stats_func(); + COMMIT; + BEGIN; + SELECT test_xact_stats_func(); + SELECT 'calls:' || calls FROM pg_stat_xact_user_functions + WHERE funcname = 'test_xact_stats_func'; + COMMIT;}, + qr/^calls:1$/m, + "function calls shows only current transaction's calls"); + +psql_c_like( + $node, + q{BEGIN; + SELECT count(*) FROM test_xact_stats; + PREPARE TRANSACTION 'test_prep'; + BEGIN; + SELECT count(*) FROM test_xact_stats; + SELECT 'seq_scan:' || seq_scan FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;}, + qr/^seq_scan:1$/m, + "table seq_scan does not leak across PREPARE boundary"); + +$node->safe_psql($dbname, q{COMMIT PREPARED 'test_prep';}); + +psql_c_like( + $node, + q{BEGIN; + SELECT count(*) FROM test_xact_stats; + INSERT INTO test_xact_stats VALUES (99); + COMMIT; + BEGIN; + SELECT 'seq_scan:' || seq_scan || ',n_tup_ins:' || n_tup_ins + FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;}, + qr/^seq_scan:0,n_tup_ins:0$/m, + "untouched relation entry shows zeroes in current transaction"); + +psql_c_like( + $node, + q{BEGIN; + SELECT test_xact_stats_func(); + COMMIT; + BEGIN; + SELECT 'found:' || count(*) FROM pg_stat_xact_user_functions + WHERE funcname = 'test_xact_stats_func'; + COMMIT;}, + qr/^found:0$/m, + "uncalled function is not visible in pg_stat_xact_user_functions"); + +psql_c_like( + $node, + q{BEGIN; + SELECT test_xact_stats_func(); + SELECT test_xact_stats_func(); + PREPARE TRANSACTION 'test_func_prep'; + BEGIN; + SELECT test_xact_stats_func(); + SELECT 'calls:' || calls FROM pg_stat_xact_user_functions + WHERE funcname = 'test_xact_stats_func'; + COMMIT;}, + qr/^calls:1$/m, + "function calls do not leak across PREPARE boundary"); + +$node->safe_psql($dbname, q{COMMIT PREPARED 'test_func_prep';}); + +psql_c_sequence_like( + $node, + [ + q{BEGIN; + INSERT INTO test_xact_stats VALUES (300); + PREPARE TRANSACTION 'test_commit_prep'; + SELECT pg_stat_force_next_flush();}, + q{COMMIT PREPARED 'test_commit_prep';}, + q{BEGIN; + SELECT 'n_tup_ins:' || n_tup_ins FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;} + ], + qr/^n_tup_ins:0$/m, + "table n_tup_ins does not leak after COMMIT PREPARED"); + +psql_c_sequence_like( + $node, + [ + q{BEGIN; + INSERT INTO test_xact_stats VALUES (400); + PREPARE TRANSACTION 'test_rollback_prep'; + SELECT pg_stat_force_next_flush();}, + q{ROLLBACK PREPARED 'test_rollback_prep';}, + q{BEGIN; + SELECT 'n_tup_ins:' || n_tup_ins FROM pg_stat_xact_user_tables + WHERE relname = 'test_xact_stats'; + COMMIT;} + ], + qr/^n_tup_ins:0$/m, + "table n_tup_ins does not leak after ROLLBACK PREPARED"); + +for my $gid ( + qw(test_prep test_func_prep test_commit_prep test_rollback_prep)) +{ + eval { $node->safe_psql($dbname, "ROLLBACK PREPARED '$gid'"); }; +} + +$node->safe_psql( + $dbname, q{ + DROP TABLE test_xact_stats; + DROP FUNCTION test_xact_stats_func(); +}); + +$node->stop; +done_testing(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 06532bf7152..eba58d860cf 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2330,6 +2330,7 @@ PgStat_EntryRefHashEntry PgStat_FetchConsistency PgStat_FunctionCallUsage PgStat_FunctionCounts +PgStat_FunctionPending PgStat_HashKey PgStat_IO PgStat_KindInfo -- 2.51.0