[PATCH v1 1/2] Add test for exported snapshot xmin race

From: "chee(dot)wooson" <chee(dot)wooson(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: "chee(dot)wooson" <chee(dot)wooson(at)gmail(dot)com>
Subject: [PATCH v1 1/2] Add test for exported snapshot xmin race
Date: 2026-07-29 10:10:44
Message-ID: 20260729101045.422679-2-chee.wooson@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Add injection points that pause horizon calculation and snapshot import at
deterministic locations. Add a TAP test that reproduces the
exported-snapshot xmin handoff race.

The test also covers both sides of the atomic handoff, repeated exports in
one transaction, and the lock-free cleanup path for an exported snapshot
that was never imported.
---
src/backend/storage/ipc/procarray.c | 13 +
src/test/modules/test_misc/meson.build | 1 +
.../test_misc/t/015_export_snapshot.pl | 394 ++++++++++++++++++
3 files changed, 408 insertions(+)
create mode 100644 src/test/modules/test_misc/t/015_export_snapshot.pl

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 60336b31803..66b394e2b4e 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1736,6 +1736,15 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
TransactionId xid;
TransactionId xmin;

+#ifdef USE_INJECTION_POINTS
+ {
+ char ip_name[64];
+
+ snprintf(ip_name, sizeof(ip_name),
+ "compute-xid-horizons-at-%d", index);
+ InjectionPointRun(ip_name, NULL);
+ }
+#endif
/* Fetch xid just once - see GetNewTransactionId */
xid = UINT32_ACCESS_ONCE(other_xids[index]);
xmin = UINT32_ACCESS_ONCE(proc->xmin);
@@ -1945,6 +1954,8 @@ GetOldestNonRemovableTransactionId(Relation rel)
{
ComputeXidHorizonsResult horizons;

+ INJECTION_POINT("get-oldest-nonremovable-txid", NULL);
+
ComputeXidHorizons(&horizons);

switch (GlobalVisHorizonKindForRel(rel))
@@ -2521,6 +2532,8 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
if (proc->databaseId != MyDatabaseId)
continue;

+ INJECTION_POINT("install-imported-xmin-before-reference", NULL);
+
/*
* Likewise, let's just make real sure its xmin does cover us.
*/
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index ee290698b31..eb48ee35d1d 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -23,6 +23,7 @@ tests += {
't/012_ddlutils.pl',
't/013_temp_obj_multisession.pl',
't/014_log_statement_max_length.pl',
+ 't/015_export_snapshot.pl',
],
# The injection points are cluster-wide, so disable installcheck
'runningcheck': false,
diff --git a/src/test/modules/test_misc/t/015_export_snapshot.pl b/src/test/modules/test_misc/t/015_export_snapshot.pl
new file mode 100644
index 00000000000..1ddae2d16a2
--- /dev/null
+++ b/src/test/modules/test_misc/t/015_export_snapshot.pl
@@ -0,0 +1,394 @@
+# Copyright (c) 2024-2026, PostgreSQL Global Development Group
+#
+# Test: reproduce the exported-snapshot xmin handoff race.
+#
+# Strategy (two-injection-point approach):
+# 1. VACUUM attaches "get-oldest-nonremovable-txid" with injection_wait
+# (PID-filtered via injection_points_set_local).
+# 2. Start VACUUM. The first ComputeXidHorizons call (from on-access
+# catalog pruning via GlobalVisUpdate) passes through unblocked.
+# 3. VACUUM reaches GetOldestNonRemovableTransactionId -> blocked at
+# "get-oldest-nonremovable-txid".
+# 4. Coordinator detects the block -> GLOBALLY attaches
+# "compute-xid-horizons-at-1" with injection_wait (no PID filter).
+# 5. Coordinator wakes VACUUM from "get-oldest-nonremovable-txid".
+# 6. VACUUM enters ComputeXidHorizons (holding ProcArrayLock shared)
+# -> blocked at "compute-xid-horizons-at-1" (after scanning importer
+# at index 0 with xmin=Invalid).
+# 7. Coordinator detects the second block -> handoff setup:
+# a. A transaction whose snapshot was exported but never imported
+# commits without waiting for ProcArrayLock.
+# b. Importer runs SET TRANSACTION SNAPSHOT (ProcArrayInstallImportedXmin
+# under ProcArrayLock shared, compatible with VACUUM's shared lock).
+# c. Source starts COMMIT. The fix makes its no-XID cleanup wait for
+# ProcArrayLock exclusive instead of clearing xmin lock-free.
+# 8. Coordinator verifies that COMMIT is waiting, then detaches + wakes
+# "compute-xid-horizons-at-1".
+# 9. VACUUM sees the source xmin before source COMMIT can clear it, so its
+# horizon remains low enough to preserve the deleted tuple.
+# 10. Importer queries -> 1 row.
+#
+# Without the fix, source COMMIT does not wait and VACUUM misses both xmin
+# values, so the lock-wait assertion and final visibility check both fail.
+#
+# Depends only on: injection_points (built-in test module)
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use Time::HiRes qw(usleep);
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# ---- Node setup ----
+my $node = PostgreSQL::Test::Cluster->new('export_race_node');
+$node->init;
+$node->append_conf('postgresql.conf',
+ "shared_preload_libraries = 'injection_points'");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points');
+
+# ---- Schema ----
+$node->safe_psql('postgres', qq{
+ CREATE TABLE race_test (id int, data text);
+ INSERT INTO race_test VALUES (1, 'should_be_visible');
+});
+
+# ---- Proc-array scan order ----
+# ComputeXidHorizons iterates over pgprocnos[], which contains only
+# client backends that called ProcArrayAdd(). System processes
+# (checkpointer, bgwriter, walwriter, etc.) are NOT in pgprocnos[].
+#
+# Ending a transaction does not remove its backend from pgprocnos[]; only
+# disconnecting does. The test only relies on importer preceding source, with
+# at least one entry between them.
+#
+# Injection point at index=1 pauses VACUUM after scanning importer(0).
+
+# ---- Connect sessions in scan order ----
+my $imp = $node->background_psql('postgres'); # index 0
+my $fill = $node->background_psql('postgres'); # index 1 (gap)
+my $src = $node->background_psql('postgres'); # index 2 (source)
+my $vac = $node->background_psql('postgres'); # index 3 (VACUUM)
+# index 4 (deleter, removed after commit)
+my $del = $node->background_psql('postgres');
+# index 5, then 4 after the deleter is removed
+my $coord = $node->background_psql('postgres');
+
+# ---- Phase 1: Prepare ----
+# Source: begin, read table, export snapshot
+$src->query("BEGIN ISOLATION LEVEL REPEATABLE READ");
+$src->query("SELECT * FROM race_test");
+my $token = $src->query("SELECT pg_export_snapshot()");
+$token =~ s/\s+//g;
+diag("exported snapshot token: $token");
+
+# Deleter: remove the row and commit.
+$del->query("DELETE FROM race_test WHERE id = 1");
+$del->query("COMMIT");
+diag("deleter committed");
+
+# Advance the XID counter so that the horizon (latestCompletedXid+1 when
+# no backend has a valid xmin) is strictly greater than the deleter's XID.
+# Each safe_psql opens a new connection, consumes one XID, and disconnects.
+for (my $i = 0; $i < 100; $i++)
+{
+ $node->safe_psql('postgres', "SELECT txid_current()");
+}
+diag("100 filler XIDs consumed");
+
+# Importer: begin (ready to import the snapshot later).
+# No query after BEGIN -> xmin stays Invalid, essential for the race.
+$imp->query("BEGIN ISOLATION LEVEL REPEATABLE READ");
+
+# ---- Phase 2: Two-injection-point race reproduction ----
+
+# Step 1: In VACUUM session, set local mode and attach first injection point.
+my $vac_pid = $vac->query("SELECT pg_backend_pid()");
+$vac_pid =~ s/\s+//g;
+diag("VACUUM PID: $vac_pid");
+
+$vac->query("SELECT injection_points_set_local()");
+$vac->query(
+ "SELECT injection_points_attach('get-oldest-nonremovable-txid', 'wait')");
+diag("VACUUM attached get-oldest-nonremovable-txid (PID-filtered)");
+
+# Step 2: Start VACUUM asynchronously.
+$vac->query_until(qr/vac_started/,
+ "\\echo vac_started\nVACUUM race_test;\n");
+diag("VACUUM started, waiting for first injection point...");
+
+# Step 3: Wait for VACUUM to be blocked at get-oldest-nonremovable-txid.
+{
+ my $blocked = 0;
+ for (my $i = 0; $i < 1800; $i++)
+ {
+ my $result = $coord->query(
+ "SELECT count(*) = 1 FROM pg_stat_activity"
+ . " WHERE pid = $vac_pid"
+ . " AND wait_event_type = 'InjectionPoint'");
+ if ($result =~ /t/)
+ {
+ $blocked = 1;
+ last;
+ }
+ usleep(100_000);
+ }
+ die "VACUUM did not reach first injection point within 180s"
+ unless $blocked;
+}
+diag("VACUUM blocked at get-oldest-nonremovable-txid");
+
+# Step 4: Coordinator GLOBALLY attaches compute-xid-horizons-at-1.
+# No set_local -> this blocks ALL backends that hit it. Only VACUUM
+# calls ComputeXidHorizons after this point (coordinator queries use
+# pg_stat_activity which doesn't call ComputeXidHorizons).
+$coord->query(
+ "SELECT injection_points_attach('compute-xid-horizons-at-1', 'wait')");
+diag("coordinator attached compute-xid-horizons-at-1 (GLOBAL)");
+
+# Step 5: Wake VACUUM from the first block.
+$coord->query(
+ "SELECT injection_points_wakeup('get-oldest-nonremovable-txid')");
+diag("woke VACUUM from get-oldest-nonremovable-txid");
+
+# Step 6: Wait for VACUUM to block at compute-xid-horizons-at-1.
+# VACUUM entered GetOldestNonRemovableTransactionId -> ComputeXidHorizons
+# -> acquired ProcArrayLock shared -> scanned index 0 (importer) ->
+# blocked at index 1 (filler).
+{
+ my $blocked = 0;
+ for (my $i = 0; $i < 1800; $i++)
+ {
+ my $result = $coord->query(
+ "SELECT count(*) = 1 FROM pg_stat_activity"
+ . " WHERE pid = $vac_pid"
+ . " AND wait_event_type = 'InjectionPoint'");
+ if ($result =~ /t/)
+ {
+ $blocked = 1;
+ last;
+ }
+ usleep(100_000);
+ }
+ die "VACUUM did not reach second injection point within 180s"
+ unless $blocked;
+}
+diag("VACUUM blocked at compute-xid-horizons-at-1 "
+ . "(inside ComputeXidHorizons)");
+
+# Step 7a: An export without any importer must retain the normal lock-free
+# no-XID cleanup path.
+$fill->query("BEGIN ISOLATION LEVEL REPEATABLE READ");
+my $unused_token = $fill->query("SELECT pg_export_snapshot()");
+$unused_token =~ s/\s+//g;
+
+my $fill_pid = $fill->query("SELECT pg_backend_pid()");
+$fill_pid =~ s/\s+//g;
+$fill->query_until(qr/unreferenced_commit_started/,
+ "\\echo unreferenced_commit_started\nCOMMIT;\n"
+ . "\\echo unreferenced_commit_done\n");
+
+my $unreferenced_finished = 0;
+for (my $i = 0; $i < 100; $i++)
+{
+ my $result = $coord->query(
+ "SELECT CASE"
+ . " WHEN state = 'idle' THEN 'done'"
+ . " WHEN wait_event_type = 'LWLock'"
+ . " AND wait_event = 'ProcArray' THEN 'blocked'"
+ . " ELSE 'running' END"
+ . " FROM pg_stat_activity WHERE pid = $fill_pid");
+ if ($result =~ /done/)
+ {
+ $unreferenced_finished = 1;
+ last;
+ }
+ last if $result =~ /blocked/;
+ usleep(100_000);
+}
+diag("unreferenced export started COMMIT while VACUUM holds ProcArrayLock");
+
+# Step 7b: Set up the actual handoff while VACUUM holds ProcArrayLock shared.
+# - importer imports the snapshot -> ProcArrayInstallImportedXmin
+# acquires ProcArrayLock shared (compatible).
+# - source starts committing -> its no-XID cleanup must wait for
+# ProcArrayLock exclusive.
+$imp->query("SET TRANSACTION SNAPSHOT '$token'");
+diag("importer installed snapshot (xmin now valid, low value)");
+
+# A later export in the same source transaction must preserve REFERENCED.
+my $second_token = $src->query("SELECT pg_export_snapshot()");
+$second_token =~ s/\s+//g;
+
+my $src_pid = $src->query("SELECT pg_backend_pid()");
+$src_pid =~ s/\s+//g;
+$src->query_until(qr/source_commit_started/,
+ "\\echo source_commit_started\nCOMMIT;\n\\echo source_commit_done\n");
+diag("source started COMMIT");
+
+my $source_waiting = 0;
+for (my $i = 0; $i < 100; $i++)
+{
+ my $result = $coord->query(
+ "SELECT wait_event_type = 'LWLock' AND wait_event = 'ProcArray'"
+ . " FROM pg_stat_activity"
+ . " WHERE pid = $src_pid");
+ if ($result =~ /t/)
+ {
+ $source_waiting = 1;
+ last;
+ }
+ last if $result eq '';
+ usleep(100_000);
+}
+
+# Step 8: Detach BOTH injection points, then wake VACUUM.
+# compute-xid-horizons-at-1 is the one VACUUM is currently blocked on.
+# get-oldest-nonremovable-txid must also be detached, because VACUUM
+# may call GetOldestNonRemovableTransactionId again (e.g., during
+# index vacuum or other internal processing), and nobody would wake it.
+$coord->query(
+ "SELECT injection_points_detach('compute-xid-horizons-at-1')");
+$coord->query(
+ "SELECT injection_points_detach('get-oldest-nonremovable-txid')");
+$coord->query(
+ "SELECT injection_points_wakeup('compute-xid-horizons-at-1')");
+diag("detached both injection points + woke VACUUM");
+
+$src->query_until(qr/source_commit_done/, "");
+$fill->query_until(qr/unreferenced_commit_done/, "");
+ok($unreferenced_finished,
+ "unreferenced export clears xmin without waiting for ProcArrayLock");
+ok($source_waiting,
+ "exporting transaction waits for ProcArrayLock while clearing xmin");
+diag("source committed after VACUUM released ProcArrayLock");
+
+# Step 9: Wait for VACUUM to finish.
+{
+ my $done = 0;
+ for (my $i = 0; $i < 1800; $i++)
+ {
+ my $result = $coord->query(
+ "SELECT count(*) = 1 FROM pg_stat_activity"
+ . " WHERE pid = $vac_pid"
+ . " AND state = 'idle'");
+ if ($result =~ /t/)
+ {
+ $done = 1;
+ last;
+ }
+ usleep(100_000);
+ }
+ die "VACUUM did not finish within 180s" unless $done;
+}
+diag("VACUUM finished");
+
+# ---- Phase 3: Check ----
+# Importer queries using the imported snapshot. The snapshot should see
+# the row if VACUUM correctly computed the horizon. With the race bug,
+# VACUUM computed a horizon that is too high (missed importer's low xmin),
+# so the tuple was removed -> importer sees 0 rows.
+my $count = $imp->query("SELECT count(*) FROM race_test");
+$count =~ s/\s+//g;
+diag("importer sees $count row(s)");
+
+is($count, 1,
+ "imported snapshot still sees the row after concurrent VACUUM")
+ or diag("BUG DETECTED: export-snapshot xmin race caused "
+ . "premature tuple removal (expected 1 row, got $count)");
+
+# ---- Cleanup ----
+$imp->query("COMMIT");
+
+# ---- Phase 4: Source wins the atomic transition ----
+# Pause an importer after it finds the source but before it changes EXPORTED
+# to REFERENCED. The source must be able to change EXPORTED to ENDING and
+# commit without ProcArrayLock; the importer must then fail.
+my $imp_fail =
+ $node->background_psql('postgres', on_error_stop => 0);
+
+$src->query("BEGIN ISOLATION LEVEL REPEATABLE READ");
+my $token2 = $src->query("SELECT pg_export_snapshot()");
+$token2 =~ s/\s+//g;
+
+$imp_fail->query("SELECT injection_points_set_local()");
+$imp_fail->query(
+ "SELECT injection_points_attach("
+ . "'install-imported-xmin-before-reference', 'wait')");
+my $imp_fail_pid = $imp_fail->query("SELECT pg_backend_pid()");
+$imp_fail_pid =~ s/\s+//g;
+$imp_fail->query("BEGIN ISOLATION LEVEL REPEATABLE READ");
+
+$imp_fail->query_until(qr/failing_import_started/,
+ "\\echo failing_import_started\nSET TRANSACTION SNAPSHOT '$token2';\n"
+ . "\\echo failing_import_done\n");
+
+{
+ my $blocked = 0;
+ for (my $i = 0; $i < 100; $i++)
+ {
+ my $result = $coord->query(
+ "SELECT wait_event_type = 'InjectionPoint'"
+ . " FROM pg_stat_activity WHERE pid = $imp_fail_pid");
+ if ($result =~ /t/)
+ {
+ $blocked = 1;
+ last;
+ }
+ usleep(100_000);
+ }
+ die "importer did not reach pre-reference injection point"
+ unless $blocked;
+}
+
+$src->query_until(qr/source_won_commit_started/,
+ "\\echo source_won_commit_started\nCOMMIT;\n"
+ . "\\echo source_won_commit_done\n");
+
+my $source_won = 0;
+for (my $i = 0; $i < 100; $i++)
+{
+ my $result = $coord->query(
+ "SELECT CASE"
+ . " WHEN state = 'idle' THEN 'done'"
+ . " WHEN wait_event_type = 'LWLock'"
+ . " AND wait_event = 'ProcArray' THEN 'blocked'"
+ . " ELSE 'running' END"
+ . " FROM pg_stat_activity WHERE pid = $src_pid");
+ if ($result =~ /done/)
+ {
+ $source_won = 1;
+ last;
+ }
+ last if $result =~ /blocked/;
+ usleep(100_000);
+}
+
+$coord->query(
+ "SELECT injection_points_detach("
+ . "'install-imported-xmin-before-reference')");
+$coord->query(
+ "SELECT injection_points_wakeup("
+ . "'install-imported-xmin-before-reference')");
+
+$src->query_until(qr/source_won_commit_done/, "");
+$imp_fail->query_until(qr/failing_import_done/, "");
+
+ok($source_won,
+ "unreferenced source changes EXPORTED to ENDING without waiting");
+like($imp_fail->{stderr},
+ qr/could not import the requested snapshot/,
+ "import fails after source wins the atomic transition");
+$imp_fail->{stderr} = '';
+$imp_fail->query("ROLLBACK");
+
+$node->stop;
+done_testing();
--
2.43.0

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message chee.wooson 2026-07-29 10:10:45 [PATCH v1 2/2] Fix exported snapshot xmin handoff race
Previous Message chee.wooson 2026-07-29 10:10:43 [PATCH v1 0/2] Fix exported snapshot xmin handoff race