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
signature.asc
Description: OpenPGP digital signature
