From 8159ebea03c60ecacde914581cab3a775589112d Mon Sep 17 00:00:00 2001
From: Scott Ray <scott@scottray.io>
Date: Sat, 1 Aug 2026 15:04:54 -0700
Subject: [PATCH v2 2/2] Fix recovery conflict resolution to account for
 imported snapshots

Make ResolveRecoveryConflictWithSnapshot call GetConflictingVirtualXIDs
until no conflict remains, fixing a wrong-results bug where, after the
first call, a backend could import a conflicting snapshot, creating a
new conflict invisible to standby recovery.
---
 src/backend/storage/ipc/procarray.c | 15 +++++++++---
 src/backend/storage/ipc/standby.c   | 38 ++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 9299bcebbda..7f9025b9579 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3351,7 +3351,8 @@ GetCurrentVirtualXIDs(TransactionId limitXmin, bool excludeXmin0,
 }
 
 /*
- * GetConflictingVirtualXIDs -- returns an array of currently active VXIDs.
+ * GetConflictingVirtualXIDs -- returns an array of currently active VXIDs
+ * considered in conflict with recovery up to limitXmin.
  *
  * Usage is limited to conflict resolution during recovery on standby servers.
  * limitXmin is supplied as either a cutoff with snapshotConflictHorizon
@@ -3380,6 +3381,13 @@ GetCurrentVirtualXIDs(TransactionId limitXmin, bool excludeXmin0,
  * but that would not be true in the case of FATAL errors lagging in array,
  * but we already know those are bogus anyway, so we skip that test.
  *
+ * That reasoning covers snapshots acquired with GetSnapshotData, which yields
+ * transaction xmins above limitXmin, but it does not cover backends that
+ * import snapshots using SET TRANSACTION SNAPSHOT, which call
+ * ProcArrayInstallImportedXmin instead.  A backend that imports a snapshot
+ * from one of the conflicting backends can acquire xmin <= limitXmin, so
+ * the conflicting VXID set can grow, but only while the set is non-empty.
+ *
  * If dbOid is valid we skip backends attached to other databases.
  *
  * Be careful to *not* pfree the result from this function. We reuse
@@ -3430,8 +3438,9 @@ GetConflictingVirtualXIDs(TransactionId limitXmin, Oid dbOid)
 			 * no snapshot currently. We hold a Share lock to avoid contention
 			 * with users taking snapshots.  That is not a problem because the
 			 * current xmin is always at least one higher than the latest
-			 * removed xid, so any new snapshot would never conflict with the
-			 * test here.
+			 * removed xid, so any snapshot acquired with GetSnapshotData
+			 * would never conflict with the test here.  See the header
+			 * comment for the caveat about imported snapshots.
 			 */
 			if (!TransactionIdIsValid(limitXmin) ||
 				(TransactionIdIsValid(pxmin) && !TransactionIdFollows(pxmin, limitXmin)))
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 29af7733948..8c05cbe0702 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -24,6 +24,7 @@
 #include "access/xlogutils.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "postmaster/startup.h"
 #include "replication/slot.h"
 #include "storage/bufmgr.h"
 #include "storage/proc.h"
@@ -489,12 +490,37 @@ ResolveRecoveryConflictWithSnapshot(TransactionId snapshotConflictHorizon,
 		return;
 
 	Assert(TransactionIdIsNormal(snapshotConflictHorizon));
-	backends = GetConflictingVirtualXIDs(snapshotConflictHorizon,
-										 locator.dbOid);
-	ResolveRecoveryConflictWithVirtualXIDs(backends,
-										   RECOVERY_CONFLICT_SNAPSHOT,
-										   WAIT_EVENT_RECOVERY_CONFLICT_SNAPSHOT,
-										   true);
+
+	/*
+	 * Scan until no conflicting VXID remains.
+	 *
+	 * GetConflictingVirtualXIDs returns the list of conflicting VXIDs that
+	 * existed while it held the ProcArrayLock.  While we wait for these VXIDs
+	 * to resolve, a backend may import a conflicting snapshot, creating a
+	 * new, conflicting VXID from one of the VXIDs in the returned list.  Any
+	 * new conflict requires a live, conflicting VXID as a source, so we loop
+	 * until GetConflictingVirtualXIDs returns empty, which means no
+	 * conflicting VXID exists and no future VXID will conflict.
+	 *
+	 * The cancellation cutoff is anchored to WAL receipt time
+	 * (GetStandbyLimitTime), not to the start of each wait, so all iterations
+	 * of this loop share a single max_standby_streaming_delay budget.
+	 */
+	for (;;)
+	{
+		backends = GetConflictingVirtualXIDs(snapshotConflictHorizon,
+											 locator.dbOid);
+
+		if (!VirtualTransactionIdIsValid(backends[0]))
+			break;
+
+		ResolveRecoveryConflictWithVirtualXIDs(backends,
+											   RECOVERY_CONFLICT_SNAPSHOT,
+											   WAIT_EVENT_RECOVERY_CONFLICT_SNAPSHOT,
+											   true);
+		/* Respond to shutdown. */
+		ProcessStartupProcInterrupts();
+	}
 
 	/*
 	 * Note that WaitExceedsMaxStandbyDelay() is not taken into account here
-- 
2.50.1 (Apple Git-155)

