Re: [PATCH v2] Fix exported snapshot xmin handoff race

Hi Peter,

Thanks for the pointers. I spent some time looking at both threads.

The standby subxact-overflow export issue seems separate from this patch: it is
about preserving the contents of a recovery snapshot during export/import.

Scott's recovery-conflict issue is much closer. My v2 makes
ProcArrayInstallImportedXmin() take ProcArrayLock exclusively, which prevents a
snapshot import from installing xmin concurrently with a procarray scan such as
GetConflictingVirtualXIDs(). However, I don't think that alone fixes Scott's
case.

Consider this sequence:

1. Standby recovery is replaying a cleanup record with snapshot conflict
horizon H.

2. Backend A has xmin <= H, so GetConflictingVirtualXIDs() returns A's VXID.

3. GetConflictingVirtualXIDs() releases ProcArrayLock. The recovery conflict
wait list is now fixed to A's VXID.

4. Before A actually ends, backend B imports A's exported snapshot.

5. With v2, B's import takes ProcArrayLock exclusively, but the procarray scan
in step 2 has already finished. So B can still install the same old xmin.

6. Recovery waits for, or cancels, only A's VXID.

7. Once A is gone, recovery can replay the cleanup record even though B is still
running with the imported snapshot.

One possible way to close that remaining hole, other than rescanning, would be
to mark source PGPROCs that have been selected as snapshot-conflict waiters by
recovery, and make ProcArrayInstallImportedXmin() reject importing from such a
source.

The intended sequence would be:

1. Standby recovery calls GetConflictingVirtualXIDs(H).

2. While holding ProcArrayLock, GetConflictingVirtualXIDs() finds backend A with
xmin <= H.

3. Before releasing ProcArrayLock, recovery sets a flag in A's PGPROC saying
that A's current VXID has been selected as a snapshot-conflict waiter.

4. Later, backend B tries to import A's exported snapshot.

5. ProcArrayInstallImportedXmin() takes ProcArrayLock exclusively and finds A as
the source VXID.

6. Because A's PGPROC says that its current VXID is already a recovery
snapshot-conflict waiter, B's import is rejected instead of installing A's
old xmin.

7. A can no longer create new conflicting importers after recovery has selected
it for conflict resolution. Once recovery has waited for or cancelled A, the
original wait list is complete.

This relies on the exclusive ProcArrayLock in ProcArrayInstallImportedXmin():
while recovery is scanning the procarray and setting the flag, a concurrent
import cannot slip through before the flag becomes visible.

I have not included that in this patch because it seems to belong to Scott's
recovery-conflict thread, and it changes the scope of the fix. But I can help
explore that direction there if people think it is preferable to rescanning.

Regards,
Chee