From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Andres Freund <andres@anarazel.de>
Cc: scott@scottray.io
Subject: Re: Snapshot export on a standby corrupts hint bits on subxact overflow
Date: Wed, 29 Jul 2026 09:36:06 +0000
Message-ID: <amnJhkudengxKX+Y@bdtpg> (raw)
In-Reply-To: <amhHJiZyFA9s70Uq@bdtpg>
References: <CAH2-WzmHVeYY=pjz9x8DhhxVjXHX0pvoQ-MdiB1Tt6=o2GTiKg@mail.gmail.com>
<amhHJiZyFA9s70Uq@bdtpg>
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.comAttachments:
[text/x-diff] v2-0001-Don-t-discard-subxip-when-exporting-a-snapshot-ta.patch (18.1K, ../amnJhkudengxKX+Y@bdtpg/2-v2-0001-Don-t-discard-subxip-when-exporting-a-snapshot-ta.patch)
download | inline diff:
From 3dc670619804794a6da2b5009cd58670e2082abb Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
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 <pg@bowt.ie>
Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
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.cindex 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.buildindex 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.plnew file mode 100644index 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
[text/x-diff] v2-0002-Read-the-in-progress-set-from-subxip-in-pg_curren.patch (11.0K, ../amnJhkudengxKX+Y@bdtpg/3-v2-0002-Read-the-in-progress-set-from-subxip-in-pg_curren.patch)
download | inline diff:
From 26eb08045d85821cdc662023b414775afae66702 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
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 <pg@bowt.ie>
Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
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.sgmlindex 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.cindex 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.plindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, pg@bowt.ie, pgsql-hackers@lists.postgresql.org, andres@anarazel.de, scott@scottray.io
Subject: Re: Snapshot export on a standby corrupts hint bits on subxact overflow
In-Reply-To: <amnJhkudengxKX+Y@bdtpg>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox