From 51ec93006f03dbacb39d033ee6313083f1861c93 Mon Sep 17 00:00:00 2001 From: Scott Ray Date: Sat, 25 Jul 2026 15:36:49 -0700 Subject: [PATCH v1 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 | 35 ++++++++++++++++++++++++----- 2 files changed, 41 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..5c957b03e6d 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -489,12 +489,35 @@ 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); + } /* * Note that WaitExceedsMaxStandbyDelay() is not taken into account here -- 2.50.1 (Apple Git-155)