Hi, On Tue, Jul 28, 2026 at 06:07:34AM +0000, Bertrand Drouvot wrote: > So, I think filtering is appropriate in both places: > > 1/ In ExportSnapshot(), do not include recovery subxip entries and committed > child XIDs at or above xmax when counting and serializing them, so unnecessary > entries do not consume the limited recovery subxip capacity. > > 2/ In pg_current_snapshot(), do not include source XIDs outside [xmin, xmax), > so that it enforces the rule regardless of how the source snapshot was > produced. >
Following our off-list discussion, PFA v2 that addresses the comments above. It adds xmax filtering, updates the documentation, and adds a few tests. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 3dc670619804794a6da2b5009cd58670e2082abb Mon Sep 17 00:00:00 2001 From: Peter Geoghegan <[email protected]> Date: Sun, 26 Jul 2026 21:17:34 -0400 Subject: [PATCH v2 1/2] Don't discard subxip when exporting a snapshot taken during recovery A snapshot taken during recovery stores its in-progress set in subxip, including every running top-level XID, and leaves xip empty. ExportSnapshot() nevertheless discarded that array whenever suboverflowed was set. An importer then treated live transactions as completed and could set incorrect HEAP_XMIN_INVALID or HEAP_XMAX_INVALID hint bits on the standby. Export overflowed recovery subxip arrays and teach ImportSnapshot() to read them. Keep the overflow flag so XidInMVCCSnapshot() still consults pg_subtrans for children already removed from KnownAssignedXids. A recovery snapshot can survive promotion, after which the importing transaction can acquire committed children and export it again. Filter recovery subxip entries and those children to [xmin, xmax) before counting and writing them. Reject a snapshot that still exceeds the import limit before pseudo-registering it, avoiding leftover cleanup state on error. Add a recovery TAP test covering the standby corruption scenario, pg_subtrans fallback, and import/re-export across promotion. Co-authored-by: Peter Geoghegan <[email protected]> Co-authored-by: Bertrand Drouvot <[email protected]> Discussion: https://postgr.es/m/CAH2-WzmHVeYY%3Dpjz9x8DhhxVjXHX0pvoQ-MdiB1Tt6%3Do2GTiKg%40mail.gmail.com --- src/backend/utils/time/snapmgr.c | 112 ++++++++- src/test/recovery/meson.build | 1 + .../recovery/t/055_standby_snapshot_export.pl | 227 ++++++++++++++++++ 3 files changed, 330 insertions(+), 10 deletions(-) 32.3% src/backend/utils/time/ 67.3% src/test/recovery/t/ diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index bc98a4361bf..48c2f56b4d9 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -548,6 +548,17 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, CurrentSnapshot->takenDuringRecovery = sourcesnap->takenDuringRecovery; /* NB: curcid should NOT be copied, it's a local matter */ + /* + * A snapshot taken during recovery keeps its in-progress set in subxip, + * including every running top-level XID. Its xmin is the oldest of them, + * so an empty subxip means that nothing was running. Catch a source that + * lost the array along the way: such a snapshot silently reports running + * transactions as no longer running. + */ + Assert(!CurrentSnapshot->takenDuringRecovery || + CurrentSnapshot->subxcnt > 0 || + CurrentSnapshot->xmin == CurrentSnapshot->xmax); + CurrentSnapshot->snapXactCompletionCount = 0; /* @@ -1118,7 +1129,9 @@ ExportSnapshot(Snapshot snapshot) TransactionId *children; ExportedSnapshot *esnap; int nchildren; + int nsubxids; int addTopXid; + bool write_subxids; StringInfoData buf; FILE *f; MemoryContext oldcxt; @@ -1161,6 +1174,47 @@ ExportSnapshot(Snapshot snapshot) */ nchildren = xactGetCommittedChildren(&children); + /* + * SnapshotData allows subxip entries outside [xmin, xmax), but they carry + * no information in an exported snapshot and cannot be represented in a + * pg_snapshot. This matters for a recovery snapshot that survives + * promotion: committed children acquired afterwards are at or above the + * old xmax. Filter such entries before counting them. + */ + if (snapshot->takenDuringRecovery) + { + nsubxids = 0; + for (int32 i = 0; i < snapshot->subxcnt; i++) + { + if (TransactionIdFollowsOrEquals(snapshot->subxip[i], + snapshot->xmin) && + TransactionIdPrecedes(snapshot->subxip[i], snapshot->xmax)) + nsubxids++; + } + for (int32 i = 0; i < nchildren; i++) + { + if (TransactionIdFollowsOrEquals(children[i], snapshot->xmin) && + TransactionIdPrecedes(children[i], snapshot->xmax)) + nsubxids++; + } + + /* + * The importer's array is bounded by the same value, so a recovery + * snapshot that does not fit cannot be represented at all. Check + * before pseudo-registering the exported snapshot, so that an error + * cannot leave cleanup state behind. + */ + if (nsubxids > GetMaxSnapshotSubxidCount()) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("cannot export a snapshot containing %d transaction IDs", + nsubxids), + errdetail("Snapshots taken during recovery record running top-level transaction IDs in the subtransaction array, which is limited to %d entries.", + GetMaxSnapshotSubxidCount()))); + } + else + nsubxids = snapshot->subxcnt + nchildren; + /* * Generate file path for the snapshot. We start numbering of snapshots * inside the transaction from 1. @@ -1211,8 +1265,8 @@ ExportSnapshot(Snapshot snapshot) * * However, it could be that our topXid is after the xmax, in which case * we shouldn't include it because xip[] members are expected to be before - * xmax. (We need not make the same check for subxip[] members, see - * snapshot.h.) + * xmax. SnapshotData does not require the same of subxip[] members (see + * snapshot.h), but the filtering above enforces it for the file format. */ addTopXid = (TransactionIdIsValid(topXid) && TransactionIdPrecedes(topXid, snapshot->xmax)) ? 1 : 0; @@ -1222,23 +1276,57 @@ ExportSnapshot(Snapshot snapshot) if (addTopXid) appendStringInfo(&buf, "xip:%u\n", topXid); + /* + * The importer has to know whether the snapshot was taken during recovery + * before it reads the subxid data, since that determines whether an + * overflowed subxip array is still meaningful. Emit it first. + */ + appendStringInfo(&buf, "rec:%u\n", snapshot->takenDuringRecovery); + /* * Similarly, we add our subcommitted child XIDs to the subxid data. Here, * we have to cope with possible overflow. + * + * Ignore the subxid array if it has overflowed, unless the snapshot was + * taken during recovery: in that case, top-level XIDs are in subxip as + * well, and we mustn't lose them. + * + * Such a snapshot usually belongs to a transaction that can have no XID, + * and hence no subcommitted children, since it was taken while the server + * was still in recovery. It can outlive promotion, though: a transaction + * on the promoted server can import one and then write. */ if (snapshot->suboverflowed || - snapshot->subxcnt + nchildren > GetMaxSnapshotSubxidCount()) + nsubxids > GetMaxSnapshotSubxidCount()) + { appendStringInfoString(&buf, "sof:1\n"); + write_subxids = snapshot->takenDuringRecovery; + } else { appendStringInfoString(&buf, "sof:0\n"); - appendStringInfo(&buf, "sxcnt:%d\n", snapshot->subxcnt + nchildren); + write_subxids = true; + } + + if (write_subxids) + { + appendStringInfo(&buf, "sxcnt:%d\n", nsubxids); for (int32 i = 0; i < snapshot->subxcnt; i++) - appendStringInfo(&buf, "sxp:%u\n", snapshot->subxip[i]); + { + if (!snapshot->takenDuringRecovery || + (TransactionIdFollowsOrEquals(snapshot->subxip[i], + snapshot->xmin) && + TransactionIdPrecedes(snapshot->subxip[i], snapshot->xmax))) + appendStringInfo(&buf, "sxp:%u\n", snapshot->subxip[i]); + } for (int32 i = 0; i < nchildren; i++) - appendStringInfo(&buf, "sxp:%u\n", children[i]); + { + if (!snapshot->takenDuringRecovery || + (TransactionIdFollowsOrEquals(children[i], snapshot->xmin) && + TransactionIdPrecedes(children[i], snapshot->xmax))) + appendStringInfo(&buf, "sxp:%u\n", children[i]); + } } - appendStringInfo(&buf, "rec:%u\n", snapshot->takenDuringRecovery); /* * Now write the text representation into a file. We first write to a @@ -1492,9 +1580,15 @@ ImportSnapshot(const char *idstr) for (i = 0; i < xcnt; i++) snapshot.xip[i] = parseXidFromText("xip:", &filebuf, path); + snapshot.takenDuringRecovery = parseIntFromText("rec:", &filebuf, path); snapshot.suboverflowed = parseIntFromText("sof:", &filebuf, path); - if (!snapshot.suboverflowed) + /* + * An overflowed subxip array carries no information and is not written + * out, except for a snapshot taken during recovery: that keeps its + * in-progress XIDs there, so it is written out and must be read back. + */ + if (!snapshot.suboverflowed || snapshot.takenDuringRecovery) { snapshot.subxcnt = xcnt = parseIntFromText("sxcnt:", &filebuf, path); @@ -1514,8 +1608,6 @@ ImportSnapshot(const char *idstr) snapshot.subxip = NULL; } - snapshot.takenDuringRecovery = parseIntFromText("rec:", &filebuf, path); - /* * Do some additional sanity checking, just to protect ourselves. We * don't trouble to check the array elements, just the most critical diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index ad0d85f4189..8f24a168614 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -63,6 +63,7 @@ tests += { 't/052_checkpoint_segment_missing.pl', 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', + 't/055_standby_snapshot_export.pl', ], }, } diff --git a/src/test/recovery/t/055_standby_snapshot_export.pl b/src/test/recovery/t/055_standby_snapshot_export.pl new file mode 100644 index 00000000000..704ac1141bc --- /dev/null +++ b/src/test/recovery/t/055_standby_snapshot_export.pl @@ -0,0 +1,227 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group +# +# Test snapshot export and import on a standby. +# +# A snapshot taken during recovery stores its in-progress set, including +# every running top-level XID, in subxip, leaving xip empty. Once it is also +# marked suboverflowed, which happens as soon as some transaction on the +# primary reports 64 subtransactions, exporting it must still write subxip +# out: an importer that loses it believes nothing at all is running between +# xmin and xmax, treats live transactions as aborted, and stamps +# HEAP_XMIN_INVALID / HEAP_XMAX_INVALID on their tuples. Those hint bits are +# then seen by every other session on the standby. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Enough XID-acquiring subtransactions to arm lastOverflowedXid on the +# standby, which happens at 64, and to overflow the primary's own subxid +# cache at 65 so that later xl_running_xacts records keep it armed. +my $nsubxacts = 80; + +# Checksums are off on purpose. With XLogHintBitIsNeeded(), +# MarkSharedBufferDirtyHint() declines to dirty a page for a hint bit set +# during recovery, so a wrong hint would live in shared buffers only. +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1, no_data_checksums => 1); +$primary->append_conf( + 'postgresql.conf', q[ +autovacuum = off +checkpoint_timeout = 1h +max_wal_size = 10GB +]); +$primary->start; + +$primary->backup('backup'); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'backup', has_streaming => 1); +$standby->append_conf( + 'postgresql.conf', q[ +hot_standby_feedback = off +max_standby_streaming_delay = -1 +]); +$standby->start; + +$primary->safe_psql( + 'postgres', q[ +CREATE TABLE victim(k int PRIMARY KEY, pad text); +INSERT INTO victim SELECT g, repeat('x', 1200) FROM generate_series(1, 40) g; +CREATE TABLE burner(i int); +]); + +# Hold the primary's removal horizon below U so that the version U deletes +# stays RECENTLY_DEAD. Otherwise the re-INSERT prunes it on the primary and +# replay of the prune record removes the evidence from the standby. +my $guard = $primary->background_psql('postgres'); +$guard->query_safe( + 'BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM victim'); + +# U deletes a row and stays open, so every snapshot taken from here on must +# report its XID as running. +my $u = $primary->background_psql('postgres'); +$u->query_safe('BEGIN'); +$u->query_safe('DELETE FROM victim WHERE k = 7'); +my $u_xid = $u->query_safe('SELECT pg_current_xact_id()'); + +# 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 +# must use pg_subtrans to map that child XID back to O. Keeping O open also +# keeps lastOverflowedXid armed on the standby. +my $o = $primary->background_psql('postgres'); +$o->query_safe('BEGIN'); +$o->query_safe('SAVEPOINT early'); +$o->query_safe('DELETE FROM victim WHERE k = 8'); +$o->query_safe('RELEASE early'); +$o->query_safe( + qq[DO \$\$ BEGIN + FOR i IN 1..$nsubxacts LOOP + 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)'); +$primary->wait_for_replay_catchup($standby); + +my $s1 = $standby->background_psql('postgres'); +$s1->query_safe('BEGIN ISOLATION LEVEL REPEATABLE READ'); +my $snap = $s1->query_safe('SELECT pg_export_snapshot()'); + +my $file = slurp_file($standby->data_dir . "/pg_snapshots/$snap"); +note("exported snapshot $snap:\n$file"); + +like($file, qr/^rec:1$/m, 'snapshot was taken during recovery'); +like($file, qr/^sof:1$/m, 'snapshot is suboverflowed'); + +# Without these the test could pass while exercising nothing: an xmax below +# U's XID answers "not running" through the plain range test instead. +my ($xmin) = $file =~ /^xmin:(\d+)$/m; +my ($xmax) = $file =~ /^xmax:(\d+)$/m; +cmp_ok($xmin, '<=', $u_xid, 'exported xmin does not follow the running XID'); +cmp_ok($u_xid, '<', $xmax, 'running XID precedes exported xmax'); + +like($file, qr/^sxcnt:[1-9]/m, + 'suboverflowed recovery snapshot exports its subxip array'); +like($file, qr/^sxp:$u_xid$/m, 'running XID is exported'); + +# Import the snapshot and read the victim page. This answers correctly, +# a transaction that is still running and one that aborted are +# indistinguishable here, but it is what writes the hint bits. +my $s2 = $standby->background_psql('postgres'); +$s2->query_safe( + qq[BEGIN ISOLATION LEVEL REPEATABLE READ; + SET TRANSACTION SNAPSHOT '$snap'; + SET enable_indexscan = off; + SET enable_bitmapscan = off; + SET enable_indexonlyscan = off]); +is($s2->query_safe('SELECT count(*) FROM victim'), + 40, 'importing backend sees its own snapshot'); +$s2->query_safe('COMMIT'); +$s1->query_safe('COMMIT'); + +# U commits and the freed key is used again. +$u->query_safe('COMMIT'); +$primary->safe_psql('postgres', + q[INSERT INTO victim VALUES (7, repeat('y', 1200))]); +$primary->wait_for_replay_catchup($standby); + +# Sessions that never touched the exported snapshot must agree with the +# primary. A stale HEAP_XMAX_INVALID on the version U deleted brings the old +# row back to life, so the key is returned twice. +is( $standby->safe_psql( + 'postgres', 'SELECT count(*) FROM victim WHERE k = 7'), + 1, + 'standby sees the re-inserted row once'); + +is( $standby->safe_psql( + 'postgres', + 'SELECT count(*) FROM (SELECT k FROM victim GROUP BY k HAVING count(*) > 1) d' + ), + 0, + 'no duplicate keys on standby'); + +my $digest = q[SELECT md5(string_agg(k::text, ',' ORDER BY k)) FROM victim]; +is( $standby->safe_psql('postgres', $digest), + $primary->safe_psql('postgres', $digest), + 'primary and standby agree on table contents'); + +# pg_snapshots is only emptied at startup and ImportSnapshot() takes rec: from +# the file rather than from RecoveryInProgress(), so a snapshot exported by a +# standby outlives promotion and keeps taking XidInMVCCSnapshot()'s recovery +# branch on a server that is no longer in recovery. +my $s3 = $standby->background_psql('postgres'); +$s3->query_safe('BEGIN ISOLATION LEVEL REPEATABLE READ'); +my $snap2 = $s3->query_safe('SELECT pg_export_snapshot()'); +my $before = $s3->query_safe('SELECT count(*) FROM victim'); + +my $file2 = slurp_file($standby->data_dir . "/pg_snapshots/$snap2"); +like($file2, qr/^sof:1$/m, + 'snapshot saved for post-promotion re-export is suboverflowed'); + +$o->query_safe('COMMIT'); +$primary->wait_for_replay_catchup($standby); + +is( $standby->safe_psql( + 'postgres', 'SELECT count(*) FROM victim WHERE k = 8'), + 0, + 'standby sees the committed subtransaction delete'); + +$standby->promote; +$standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()') + or die "standby never finished promotion"; + +my $s4 = $standby->background_psql('postgres'); +$s4->query_safe( + qq[BEGIN ISOLATION LEVEL REPEATABLE READ; + SET TRANSACTION SNAPSHOT '$snap2']); +is($s4->query_safe('SELECT count(*) FROM victim'), + $before, 'recovery-taken snapshot still imports after promotion'); + +# That importing transaction is read-write, since the read-only requirement +# binds only a SERIALIZABLE source. So it can acquire subcommitted children +# and export again, which hands ExportSnapshot() a snapshot taken during +# recovery together with a non-empty children array. +$s4->query_safe('SAVEPOINT sp'); +$s4->query_safe(q[INSERT INTO victim VALUES (5000, repeat('z', 10))]); +$s4->query_safe('RELEASE sp'); +my $snap3 = $s4->query_safe('SELECT pg_export_snapshot()'); + +my $file3 = slurp_file($standby->data_dir . "/pg_snapshots/$snap3"); +my ($xmin3) = $file3 =~ /^xmin:(\d+)$/m; +my ($xmax3) = $file3 =~ /^xmax:(\d+)$/m; +my @subxids3 = $file3 =~ /^sxp:(\d+)$/mg; +like($file3, qr/^sof:1$/m, + 're-exported recovery snapshot remains suboverflowed'); +like($file3, qr/^sxcnt:[1-9]/m, + 'a recovery-taken snapshot re-exports its subxip array after a write'); +is(scalar(grep { $_ < $xmin3 || $_ >= $xmax3 } @subxids3), + 0, 're-exported snapshot omits XIDs outside its range'); + +my $s5 = $standby->background_psql('postgres'); +$s5->query_safe( + qq[BEGIN ISOLATION LEVEL REPEATABLE READ; + SET TRANSACTION SNAPSHOT '$snap3']); +is($s5->query_safe('SELECT count(*) FROM victim'), + $before, 're-exported recovery snapshot can be imported'); + +$s5->query_safe('COMMIT'); +$s4->query_safe('COMMIT'); +$s3->query_safe('COMMIT'); + +$guard->quit; +$u->quit; +$o->quit; +$s1->quit; +$s2->quit; +$s3->quit; +$s4->quit; +$s5->quit; +$standby->stop; +$primary->stop; + +done_testing(); -- 2.34.1
>From 26eb08045d85821cdc662023b414775afae66702 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan <[email protected]> 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 <[email protected]> Co-authored-by: Bertrand Drouvot <[email protected]> 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} <para> Returns a current <firstterm>snapshot</firstterm>, 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 <xref linkend="subxacts"/> - 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 <xref linkend="subxacts"/> for details. </para></entry> </row> @@ -3086,8 +3087,10 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres} ID that is <literal>xmin <= <replaceable>X</replaceable> < xmax</literal> 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. </entry> </row> </tbody> 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
