postgres.git / summary / log / commit / refs
commit 79b101486c1d792600b79f90579b788385e85878
Author: Peter Geoghegan <pg@bowt.ie>
Date: Wed Aug 26 19:38:17 2026 +0000
Export subxip[] for snapshots taken during recovery.
A snapshot taken during recovery stores all of its in-progress XIDs in
subxip, every running top-level XID included, leaving xip empty. Unlike
with other snapshots, its suboverflowed flag does not mean that subxip
is redundant. We nevertheless treated it that way during snapshot
export, so an importing session could see in-progress transactions as
aborted. This misbehavior could also lead to hint bits being
incorrectly set on the standby; affected tuples then wrongly appeared
visible or invisible to sessions that never imported the snapshot.
To fix, teach snapshot export to include the subxip[] array regardless
of the overflow flag when the snapshot is taken during recovery. This
is in line with how CopySnapshot() and SerializeSnapshot() already
handle the same issue.
Claude Code diagnosed this problem. The committed TAP test is a
simplified version of the one that it wrote to demonstrate this bug.
Oversight in commit 6c2003f8a, which enabled snapshot export and import
during recovery.
Author: Peter Geoghegan <pg@bowt.ie>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Bug: #17846
Discussion: https://postgr.es/m/CAH2-WzmHVeYY%3Dpjz9x8DhhxVjXHX0pvoQ-MdiB1Tt6%3Do2GTiKg%40mail.gmail.com
Discussion: https://postgr.es/m/17846-1a0e5ce976f4c01a@postgresql.org
Backpatch-through: 14
src/backend/utils/time/snapmgr.c | 58 ++++++++----
src/test/recovery/meson.build | 1 +
src/test/recovery/t/056_standby_snapshot_export.pl | 104 +++++++++++++++++++++
3 files changed, 147 insertions(+), 16 deletions(-)
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 70aed370aa0..6a19dcfb2e0 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1117,8 +1117,10 @@ ExportSnapshot(Snapshot snapshot)
TransactionId topXid;
TransactionId *children;
ExportedSnapshot *esnap;
+ int nsubxids;
int nchildren;
int addTopXid;
+ bool suboverflowed;
StringInfoData buf;
FILE *f;
MemoryContext oldcxt;
@@ -1161,6 +1163,22 @@ ExportSnapshot(Snapshot snapshot)
*/
nchildren = xactGetCommittedChildren(&children);
+ /*
+ * We export a recovery snapshot's subxip whole (see below), so refuse an
+ * export that no importer will accept. This rare edge case only happens
+ * when a snapshot taken during recovery is imported after its standby is
+ * promoted, the importing transaction subcommits many subtransactions,
+ * and then attempts to export the same snapshot a second time.
+ */
+ if (snapshot->takenDuringRecovery &&
+ snapshot->subxcnt + nchildren > GetMaxSnapshotSubxidCount())
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("cannot export snapshot with %d running transaction IDs",
+ snapshot->subxcnt + nchildren),
+ errdetail("A snapshot taken during recovery is exported with every transaction ID that it treats as running, and at most %d can be stored.",
+ GetMaxSnapshotSubxidCount())));
+
/*
* Generate file path for the snapshot. We start numbering of snapshots
* inside the transaction from 1.
@@ -1223,16 +1241,29 @@ ExportSnapshot(Snapshot snapshot)
appendStringInfo(&buf, "xip:%u\n", topXid);
/*
- * Similarly, we add our subcommitted child XIDs to the subxid data. Here,
- * we have to cope with possible overflow.
+ * Similarly, we add our subcommitted child XIDs to the subxid data.
+ *
+ * Report overflow when the snapshot overflowed, and also when our subxids
+ * won't fit in what a snapshot can hold. For a snapshot taken outside
+ * recovery, claiming overflow is always safe, since it just makes
+ * importers fall back on pg_subtrans.
*/
- if (snapshot->suboverflowed ||
- snapshot->subxcnt + nchildren > GetMaxSnapshotSubxidCount())
- appendStringInfoString(&buf, "sof:1\n");
- else
+ nsubxids = snapshot->subxcnt + nchildren;
+ suboverflowed = snapshot->suboverflowed ||
+ nsubxids > GetMaxSnapshotSubxidCount();
+
+ /*
+ * 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.
+ */
+ if (suboverflowed && !snapshot->takenDuringRecovery)
+ nsubxids = 0;
+
+ appendStringInfo(&buf, "sof:%u\n", suboverflowed);
+ appendStringInfo(&buf, "sxcnt:%d\n", nsubxids);
+ if (nsubxids > 0)
{
- appendStringInfoString(&buf, "sof:0\n");
- appendStringInfo(&buf, "sxcnt:%d\n", snapshot->subxcnt + nchildren);
for (int32 i = 0; i < snapshot->subxcnt; i++)
appendStringInfo(&buf, "sxp:%u\n", snapshot->subxip[i]);
for (int32 i = 0; i < nchildren; i++)
@@ -1493,11 +1524,11 @@ ImportSnapshot(const char *idstr)
snapshot.xip[i] = parseXidFromText("xip:", &filebuf, path);
snapshot.suboverflowed = parseIntFromText("sof:", &filebuf, path);
+ snapshot.subxcnt = xcnt = parseIntFromText("sxcnt:", &filebuf, path);
+ snapshot.subxip = NULL;
- if (!snapshot.suboverflowed)
+ if (snapshot.subxcnt)
{
- snapshot.subxcnt = xcnt = parseIntFromText("sxcnt:", &filebuf, path);
-
/* sanity-check the xid count before palloc */
if (xcnt < 0 || xcnt > GetMaxSnapshotSubxidCount())
ereport(ERROR,
@@ -1508,11 +1539,6 @@ ImportSnapshot(const char *idstr)
for (i = 0; i < xcnt; i++)
snapshot.subxip[i] = parseXidFromText("sxp:", &filebuf, path);
}
- else
- {
- snapshot.subxcnt = 0;
- snapshot.subxip = NULL;
- }
snapshot.takenDuringRecovery = parseIntFromText("rec:", &filebuf, path);
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 39ec8c4946d..72113c5ac6e 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -64,6 +64,7 @@ tests += {
't/053_standby_login_event_trigger.pl',
't/054_unlogged_sequence_promotion.pl',
't/055_cascade_reconnect.pl',
+ 't/056_standby_snapshot_export.pl',
],
},
}
diff --git a/src/test/recovery/t/056_standby_snapshot_export.pl b/src/test/recovery/t/056_standby_snapshot_export.pl
new file mode 100644
index 00000000000..73abb9c49b8
--- /dev/null
+++ b/src/test/recovery/t/056_standby_snapshot_export.pl
@@ -0,0 +1,104 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Test snapshot export and import on a standby.
+#
+# A snapshot taken during recovery holds its whole in-progress set in subxip,
+# so export must write subxip out even when the snapshot is suboverflowed.
+# Otherwise, a session that imports the snapshot treats running transactions
+# as aborted, incorrectly setting hint bits.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# We must use at least enough subxacts to overflow the primary's subxid cache
+my $nsubxacts = 80;
+
+my $primary = PostgreSQL::Test::Cluster->new('primary');
+$primary->init(allows_streaming => 1);
+$primary->append_conf('postgresql.conf', 'autovacuum = off');
+$primary->start;
+
+$primary->backup('backup');
+my $standby = PostgreSQL::Test::Cluster->new('standby');
+$standby->init_from_backup($primary, 'backup', has_streaming => 1);
+$standby->start;
+
+$primary->safe_psql(
+ 'postgres', q[
+CREATE TABLE vistest AS SELECT g AS k FROM generate_series(1, 10) g;
+CREATE TABLE xid_burner(i int);
+]);
+
+# This transaction deletes a row and stays open, so every snapshot taken from
+# here on must report its XID as running
+my $deleter = $primary->background_psql('postgres');
+$deleter->query_safe('BEGIN');
+$deleter->query_safe('DELETE FROM vistest WHERE k = 7');
+my $deleter_xid = $deleter->query_safe('SELECT pg_current_xact_id()');
+
+# This one deletes another row in an early subtransaction, then overflows its
+# subxid cache and stays open. Recovery removes the deleting subtransaction's
+# XID from KnownAssignedXids, so reaching that tuple's xmax has to map the
+# child XID back to its parent through pg_subtrans.
+my $subxact_deleter = $primary->background_psql('postgres');
+$subxact_deleter->query_safe('BEGIN');
+$subxact_deleter->query_safe('SAVEPOINT early');
+$subxact_deleter->query_safe('DELETE FROM vistest WHERE k = 8');
+$subxact_deleter->query_safe('RELEASE early');
+
+# Burn $nsubxacts-many subxact XIDs to make exported snapshot suboverflowed
+$subxact_deleter->query_safe(
+ qq[DO \$\$ BEGIN
+ FOR i IN 1..$nsubxacts LOOP
+ BEGIN INSERT INTO xid_burner VALUES (i);
+ EXCEPTION WHEN OTHERS THEN NULL; END;
+ END LOOP; END \$\$]);
+
+# Commit a transaction that writes WAL of its own. That advances
+# latestCompletedXid on the standby past the deleting XID, and flushes the
+# xid-assignment WAL that those subtransactions wrote.
+$primary->safe_psql('postgres', 'INSERT INTO xid_burner VALUES (0)');
+$primary->wait_for_replay_catchup($standby);
+
+my $exporter = $standby->background_psql('postgres');
+$exporter->query_safe('BEGIN ISOLATION LEVEL REPEATABLE READ');
+my $snap = $exporter->query_safe('SELECT pg_export_snapshot()');
+
+my $snapfile = slurp_file($standby->data_dir . "/pg_snapshots/$snap");
+note("exported snapshot $snap:\n$snapfile");
+
+like($snapfile, qr/^rec:1$/m, 'snapshot was taken during recovery');
+like($snapfile, qr/^sof:1$/m, 'snapshot is suboverflowed');
+
+my ($xmin) = $snapfile =~ /^xmin:(\d+)$/m;
+my ($xmax) = $snapfile =~ /^xmax:(\d+)$/m;
+ok( $xmin <= $deleter_xid && $deleter_xid < $xmax,
+ 'running XID falls inside the exported xmin/xmax range');
+
+like($snapfile, qr/^sxp:$deleter_xid$/m,
+ 'running XID appears in exported subxip array');
+
+# Let both deleters commit, and let the standby replay that
+$deleter->query_safe('COMMIT');
+$subxact_deleter->query_safe('COMMIT');
+$primary->wait_for_replay_catchup($standby);
+
+is( $standby->safe_psql(
+ 'postgres', qq[BEGIN ISOLATION LEVEL REPEATABLE READ;
+ SET TRANSACTION SNAPSHOT '$snap';
+ SELECT count(*) FROM vistest]),
+ 10,
+ 'imported recovery snapshot still sees the deleted rows');
+
+$exporter->query_safe('COMMIT');
+
+$subxact_deleter->quit;
+$deleter->quit;
+$exporter->quit;
+$standby->stop;
+$primary->stop;
+
+done_testing();
[parent: f3b0bb29834e]