Re: Recovery conflict resolution misses backends that import snapshots

Thanks for posting v3.

1. v3 still exposes snapshot-importing backends to wrong results.
Consider procs Exporter, Recovery, and Importer:

(a) Recovery scans, sets Exporter's recoveryConflictTracked to 1, and
begins to wait for the transaction to end using VirtualXactLock().

(b) Importer calls ProcArrayInstallImportedXmin(), acquires the
ProcArrayLock exclusively, and stalls just before reading
recoveryConflictTracked, for example because the OS preempts it.

(c) Exporter commits and takes the lockless branch of
ProcArrayEndTransaction().

(d) Recovery sees Exporter's transaction end, calls
ProcArrayClearRecoveryConflictTracked(), and resumes replay.

(e) Importer resumes, reads that recoveryConflictTracked contains 0,
and finishes importing the snapshot.

(f) Importer now holds a snapshot that needs tuple versions or index
entries that Recovery has removed.

Holding ProcArrayLock in shared mode while clearing
recoveryConflictTracked would prevent this race.

2. v3 can block importing snapshots even when the snapshots do not
conflict with recovery. Consider procs Exporter, Recovery, Importer,
and Blocker:

(a) Recovery scans, sets Exporter and Blocker's
recoveryConflictTracked to 1, and begins to wait for Blocker.

(b) Exporter's transaction ends and it begins a new transaction with
a new snapshot that does not conflict.

(c) Importer attempts to import Exporter's new snapshot, but
recoveryConflictTracked is still 1, and so import fails. The user
receives the false error detail "The source process with PID %d is not
running anymore."

(d) This condition persists until Blocker's transaction completes and
Recovery reaches Exporter's old VXID in the waitlist, at which point
it calls ProcArrayClearRecoveryConflictTracked() and import may
succeed.

GetConflictingVirtualXIDs() could store the lxid of the conflicting
transaction and ProcArrayInstallImportedXmin() could check this value to
determine whether it matches the source of the import. The false error
detail requires a separate fix.

3. This approach can cause parallel pg_dump failures.

(a) The leader connects, opens a transaction, and exports its
snapshot. It does not immediately fork workers. Instead, it
continues preparing, including a full scan of the catalog to find
every table to dump.

(b) GetConflictingVirtualXIDs() sets recoveryConflictTracked to 1 for
the leader.

(c) The leader finishes setting up and forks workers.

(d) Each worker attempts to import the leader's snapshot but fails,
and pg_dump exits with "a worker process died unexpectedly".

The user may set max_standby_streaming_delay to -1 to prioritize
operations on the standby over recovery progress. Raising an error if
a proc tries to import a conflicting snapshot lets the operation die
instead of delaying recovery, regardless of the user's stated
preference.

4. v3 offers no clear backpatch strategy. At its current location,
recoveryConflictTracked breaks ABI compatibility by displacing PGPROC
members including lwWaiting. Also, some of the components v3 uses
postdate affected, supported branches: PG 14 through 16 lack injection
points; and no stable release uses RecoveryConflictReason.

--
Scott Ray