Re: Recovery conflict resolution misses backends that import snapshots

Поиск
Список
Период
Сортировка
Искать

Re: Recovery conflict resolution misses backends that import snapshots

От:
"chee.wooson" <chee.wooson@gmail.com>
Дата:
Hi,

Based on the discussion, I tried an alternative approach for master that
avoids repeatedly rescanning the procarray.

Recovery currently collects a fixed list of VXIDs whose xmins conflict
with a cleanup WAL record.  After ProcArrayLock is released, another
backend can import a listed backend's snapshot and advertise the same
xmin.  The importer is not in recovery's wait list, so recovery can
finish waiting and replay the cleanup while the imported snapshot still
needs the removed data.

The attached v3 uses a separate atomic recoveryConflictTracked field in
PGPROC.  Startup marks each conflicting snapshot source while collecting
the wait list under shared ProcArrayLock.  ProcArrayInstallImportedXmin()
holds ProcArrayLock exclusively and rejects imports from marked sources.

This gives the following ordering:

- An import completed before the scan is visible to the scan and included
  in the wait list.
- An import attempted after the scan observes the source marker and fails.

ResolveRecoveryConflictWithVirtualXIDs() clears each marker immediately
after the corresponding VXID finishes.  The marker is used only for
RECOVERY_CONFLICT_SNAPSHOT.  It is separate from pendingRecoveryConflicts
because the cancellation bits have a different lifetime and are consumed
by backend interrupt processing.

Compared with v2, this prevents the chain of conflicting importers from
growing instead of rescanning until no conflicts remain.  It also retains
the boolean return value of ProcArrayInstallImportedXmin(), and the new
field does not need explicit initialization in ProcGlobalShmemInit, just
as pendingRecoveryConflicts does not.

The attached series is:

- v3-0001 adds a deterministic TAP reproducer and its injection points.
- v3-0002 implements the recoveryConflictTracked protocol.

Patch 0001 is expected to fail without patch 0002 because the conflicting
snapshot import succeeds.

The series is based on master at 9f4bd91a196.  I tested it with assertions,
injection points, and TAP tests enabled.  The build completed successfully,
and recovery tests 056_standby_snapshot_export and
057_snapshot_import_conflict passed.  The series also applies cleanly to
that master commit.

This approach conservatively rejects all snapshot imports from a tracked
source until its tracked VXID finishes.  Feedback on this tradeoff and the
marker lifetime would be appreciated.

Regards,
Chee

Re: Recovery conflict resolution misses backends that import snapshots

От:
Scott Ray <scott@scottray.io>
Дата:
On Monday, July 27th, 2026 at 9:57 PM, Amit Kapila  wrote:

> I think we should at least have CFI in this loop so that it responds
> to promotion, shutdown, etc.

If the goal is to make the standby responsive to shutdown, we should
add ProcessStartupProcInterrupts(), which is already called elsewhere
during redo: xlog_redo() -> CheckRequiredParameterValues() ->
RecoveryRequiresIntParameter() -> ProcessStartupProcInterrupts()
in a loop.  CHECK_FOR_INTERRUPTS() doesn't handle shutdown.

If the target is promotion, then we could use CheckForStandbyTrigger().
Conflict resolution occurs while the standby is processing a record,
and from what I can tell, responding to promotion after beginning to
apply the record but before finishing is unprecedented and would force
the standby to decide what to do with the partially-applied record.

The attached v2 calls ProcessStartupProcInterrupts(), and I confirmed
that the standby shuts down promptly when signaled.

On Mon, Jul 27, 2026 at 5:19 AM Michael Paquier
 wrote:
>
> Adding a code pattern that could potentially cause this code path to
> loop infinitely is not what I would call a principled approach, I
> would call it a risky one.

I tried to cause an infinite loop using a standby with
max_standby_streaming_delay = 5s, max_connections = 400, and a pool of
clients that relays one old snapshot forward as fast as it can:

t= 1.1s  212 conflicting VXIDs
t= 5.5s  363 population stops growing, no connection slots left
t= 6.6s  277 cutoff has passed, cancellation begins
t= 7.7s  190
t= 8.8s  105
t= 9.9s   16
t=11.0s    0 replay resumes

The standby begins killing VXIDs and kills them too fast for a
sustained relay - at least on my machine with this setup.  The standby
calls pg_usleep(5000) after each signal, which is why the
population remains above 0 for several seconds after the cutoff.

--
Scott Ray
FAQ