From 26eb08045d85821cdc662023b414775afae66702 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Sun, 26 Jul 2026 21:18:45 -0400 Subject: [PATCH v2 2/2] Read the in-progress set from subxip in pg_current_snapshot() pg_current_snapshot() copied the active snapshot's xip array. A snapshot taken during recovery keeps its in-progress XIDs in subxip and leaves xip empty, so the function reported every transaction in [xmin, xmax) as completed. pg_visible_in_snapshot() could therefore disagree with tuple visibility on a standby. No subxid overflow is required. Read the array populated by recovery and filter it to [xmin, xmax), as required by pg_snapshot. Account for the larger recovery array in the compile-time allocation bound. Recovery cannot distinguish top-level and subtransaction XIDs, so update the source comments and documentation to describe the subtransaction IDs that can appear in recovery snapshots. Test the behavior without overflow through both pg_current_snapshot() and the legacy txid_current_snapshot() interface. Also verify both interfaces after importing a recovery snapshot across promotion, including range validity and textual pg_snapshot input. Co-authored-by: Peter Geoghegan Co-authored-by: Bertrand Drouvot Discussion: https://postgr.es/m/CAH2-WzmHVeYY%3Dpjz9x8DhhxVjXHX0pvoQ-MdiB1Tt6%3Do2GTiKg%40mail.gmail.com --- doc/src/sgml/func/func-info.sgml | 13 ++-- src/backend/utils/adt/xid8funcs.c | 66 ++++++++++++++----- .../recovery/t/055_standby_snapshot_export.pl | 55 ++++++++++++++-- 3 files changed, 107 insertions(+), 27 deletions(-) 14.5% doc/src/sgml/func/ 49.5% src/backend/utils/adt/ 35.8% src/test/recovery/t/ diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml index 122fc740f1a..bbde54ff0fa 100644 --- a/doc/src/sgml/func/func-info.sgml +++ b/doc/src/sgml/func/func-info.sgml @@ -2905,9 +2905,10 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} Returns a current snapshot, a data structure showing which transaction IDs are now in-progress. - Only top-level transaction IDs are included in the snapshot; - subtransaction IDs are not shown; see - for details. + Normally, only top-level transaction IDs are included in the snapshot. + A snapshot taken during recovery can also include + subtransaction IDs because recovery cannot distinguish them from + top-level transaction IDs. See for details. @@ -3086,8 +3087,10 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} ID that is xmin <= X < xmax and not in this list was already completed at the time of the snapshot, and thus is either visible or dead according to its - commit status. This list does not include the transaction IDs of - subtransactions (subxids). + commit status. This list normally does not include the transaction IDs + of subtransactions (subxids), but a snapshot taken during recovery can + include them because recovery cannot distinguish subtransaction IDs + from top-level transaction IDs. diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index c607e78d9ac..0a398c584e6 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -3,11 +3,13 @@ * * Export internal transaction IDs to user level. * - * Note that only top-level transaction IDs are exposed to user sessions. - * This is important because xid8s frequently persist beyond the global - * xmin horizon, or may even be shipped to other machines, so we cannot - * rely on being able to correlate subtransaction IDs with their parents - * via functions such as SubTransGetTopmostTransaction(). + * Normally only top-level transaction IDs are exposed to user sessions. + * Snapshots taken during recovery can also expose subtransaction IDs, since + * recovery cannot distinguish them from top-level IDs. xid8s frequently + * persist beyond the global xmin horizon, or may even be shipped to other + * machines, so callers cannot rely on being able to correlate subtransaction + * IDs with their parents via functions such as + * SubTransGetTopmostTransaction(). * * These functions are used to support the txid_XXX functions and the newer * pg_current_xact_id, pg_current_snapshot and related fmgr functions, since @@ -33,6 +35,7 @@ #include "libpq/pqformat.h" #include "miscadmin.h" #include "storage/lwlock.h" +#include "storage/proc.h" #include "storage/procarray.h" #include "storage/procnumber.h" #include "utils/builtins.h" @@ -75,9 +78,11 @@ typedef struct /* * Compile-time limits on the procarray (MAX_BACKENDS processes plus - * MAX_BACKENDS prepared transactions) guarantee nxip won't be too large. + * MAX_BACKENDS prepared transactions) and the number of subxids cached for + * each process guarantee nxip won't be too large. */ -StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP, +StaticAssertDecl((PGPROC_MAX_CACHED_SUBXIDS + 1) * MAX_BACKENDS * 2 <= + PG_SNAPSHOT_MAX_NXIP, "possible overflow in pg_current_snapshot()"); @@ -365,37 +370,62 @@ pg_current_xact_id_if_assigned(PG_FUNCTION_ARGS) * * Return current snapshot * - * Note that only top-transaction XIDs are included in the snapshot. + * Snapshots taken during recovery can also include subtransaction XIDs because + * recovery cannot distinguish them from top-level XIDs. */ Datum pg_current_snapshot(PG_FUNCTION_ARGS) { pg_snapshot *snap; - uint32 nxip, + uint32 nxip = 0, + source_nxip, i; Snapshot cur; + TransactionId *xip; FullTransactionId next_fxid = ReadNextFullTransactionId(); cur = GetActiveSnapshot(); if (cur == NULL) elog(ERROR, "no active snapshot set"); + /* + * A snapshot taken during recovery stores its in-progress XIDs in subxip + * and leaves xip empty, so read the in-progress set from there. Reading + * xip would report every running transaction as already completed. + */ + if (cur->takenDuringRecovery) + { + source_nxip = cur->subxcnt; + xip = cur->subxip; + } + else + { + source_nxip = cur->xcnt; + xip = cur->xip; + } + /* allocate */ - nxip = cur->xcnt; - snap = palloc(PG_SNAPSHOT_SIZE(nxip)); + snap = palloc(PG_SNAPSHOT_SIZE(source_nxip)); /* - * Fill. This is the current backend's active snapshot, so MyProc->xmin - * is <= all these XIDs. As long as that remains so, oldestXid can't - * advance past any of these XIDs. Hence, these XIDs remain allowable - * relative to next_fxid. + * Fill. Unlike SnapshotData's subxip, pg_snapshot's xip cannot contain + * XIDs outside [xmin, xmax), so filter the source at this boundary. This + * is the current backend's active snapshot, so MyProc->xmin protects all + * retained XIDs from oldestXid. Hence, they remain allowable relative to + * next_fxid. */ snap->xmin = FullTransactionIdFromAllowableAt(next_fxid, cur->xmin); snap->xmax = FullTransactionIdFromAllowableAt(next_fxid, cur->xmax); + for (i = 0; i < source_nxip; i++) + { + if (TransactionIdPrecedes(xip[i], cur->xmin) || + TransactionIdFollowsOrEquals(xip[i], cur->xmax)) + continue; + + snap->xip[nxip++] = + FullTransactionIdFromAllowableAt(next_fxid, xip[i]); + } snap->nxip = nxip; - for (i = 0; i < nxip; i++) - snap->xip[i] = - FullTransactionIdFromAllowableAt(next_fxid, cur->xip[i]); /* * We want them guaranteed to be in ascending order. This also removes diff --git a/src/test/recovery/t/055_standby_snapshot_export.pl b/src/test/recovery/t/055_standby_snapshot_export.pl index 704ac1141bc..898bacefd8e 100644 --- a/src/test/recovery/t/055_standby_snapshot_export.pl +++ b/src/test/recovery/t/055_standby_snapshot_export.pl @@ -66,6 +66,33 @@ $u->query_safe('BEGIN'); $u->query_safe('DELETE FROM victim WHERE k = 7'); my $u_xid = $u->query_safe('SELECT pg_current_xact_id()'); +# Commit something so that the standby's xmax ends up past U's XID. Without +# this, the visibility functions answer through their xid >= xmax range test +# and the test would pass without examining the in-progress array. +$primary->safe_psql('postgres', 'INSERT INTO burner VALUES (0)'); +$primary->wait_for_replay_catchup($standby); + +# A recovery snapshot stores its in-progress XIDs in subxip and leaves xip +# empty. Verify the SQL interface before creating any subxid overflow. +is( $standby->safe_psql( + 'postgres', + "SELECT pg_visible_in_snapshot('$u_xid'::xid8, pg_current_snapshot())" + ), + 'f', + 'pg_current_snapshot reports a running XID as not visible'); + +is( $standby->safe_psql( + 'postgres', + "SELECT txid_visible_in_snapshot('$u_xid'::bigint, txid_current_snapshot())" + ), + 'f', + 'txid_current_snapshot reports a running XID as not visible'); + +is( $standby->safe_psql( + 'postgres', 'SELECT count(*) FROM victim WHERE k = 7'), + 1, + 'the running transaction has not deleted its row'); + # O deletes another row in an early subtransaction, then overflows the subxid # cache and stays open. Recovery removes the deleting subtransaction's XID # from KnownAssignedXids, so a later visibility check of the tuple's xmax @@ -82,10 +109,10 @@ $o->query_safe( BEGIN INSERT INTO burner VALUES (i); EXCEPTION WHEN OTHERS THEN NULL; END; END LOOP; END \$\$]); -# Commit something so that the standby's xmax ends up past U's XID. Without -# this, XidInMVCCSnapshot() answers via its xid >= xmax range test and the -# test would pass without exercising anything. -$primary->safe_psql('postgres', 'INSERT INTO burner VALUES (0)'); +# Commit after O has overflowed to flush its preceding xid-assignment WAL, then +# wait until the standby has removed those subxids and marked snapshots +# overflowed. +$primary->safe_psql('postgres', 'INSERT INTO burner VALUES (-1)'); $primary->wait_for_replay_catchup($standby); my $s1 = $standby->background_psql('postgres'); @@ -209,6 +236,26 @@ $s5->query_safe( is($s5->query_safe('SELECT count(*) FROM victim'), $before, 're-exported recovery snapshot can be imported'); +is( $s5->query_safe( + 'SELECT count(*) ' + . 'FROM pg_snapshot_xip(pg_current_snapshot()) AS x(xid) ' + . 'WHERE xid < pg_snapshot_xmin(pg_current_snapshot()) ' + . 'OR xid >= pg_snapshot_xmax(pg_current_snapshot())'), + 0, + 'pg_current_snapshot has no explicit XIDs outside its range'); + +is( $s5->query_safe( + 'SELECT count(*) ' + . 'FROM txid_snapshot_xip(txid_current_snapshot()) AS x(xid) ' + . 'WHERE xid < txid_snapshot_xmin(txid_current_snapshot()) ' + . 'OR xid >= txid_snapshot_xmax(txid_current_snapshot())'), + 0, + 'txid_current_snapshot has no explicit XIDs outside its range'); + +my $current_snapshot = $s5->query_safe('SELECT pg_current_snapshot()::text'); +is($s5->query_safe("SELECT '$current_snapshot'::pg_snapshot IS NOT NULL"), + 't', 'pg_current_snapshot output is valid pg_snapshot input'); + $s5->query_safe('COMMIT'); $s4->query_safe('COMMIT'); $s3->query_safe('COMMIT'); -- 2.34.1