Re: Recovery conflict resolution misses backends that import snapshots - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Recovery conflict resolution misses backends that import snapshots
Date
Msg-id CAA4eK1K09ZZWF-5xxVRL=XMwg=COGCRM8X2bFJJaoYkucXB8PA@mail.gmail.com
Whole thread
In response to Re: Recovery conflict resolution misses backends that import snapshots  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Recovery conflict resolution misses backends that import snapshots
List pgsql-hackers
On Mon, Jul 27, 2026 at 5:19 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> +       /*
> +        * Scan until no conflicting VXID remains.
> [...]
> +       for (;;)
> +       {
>
> 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 think we should at least have CFI in this loop so that it responds
to promotion, shutdown, etc. Currently, it only happens on the waiting
path: ResolveRecoveryConflictWithVirtualXIDs() → inner while
(!VirtualXactLock(*waitlist, false)) → WaitExceedsMaxStandbyDelay(),
which calls CHECK_FOR_INTERRUPTS(). But if a scan returns a non-empty
list and, by the time you call
ResolveRecoveryConflictWithVirtualXIDs(), all those VXIDs have already
ended, VirtualXactLock(..., false) returns true immediately for each
entry, the wait branch is never taken, and that whole call does zero
CFI.

For a finite value of max_standby_streaming_delay, the deadline is
anchored to WAL receipt and does not reset per iteration as mentioned
in the comment in the patch, so once it passes,
ResolveRecoveryConflictWithVirtualXIDs starts cancelling the
conflicting backends (SignalRecoveryConflictWithVirtualXID), not just
waiting. Recovery then makes forward progress by killing sources. A
new importer must find a source that's still alive in a window that
recovery is actively shrinking by terminating everything in it, and
each new importer is itself immediately cancellable on the next pass
(deadline already elapsed → cancel now). So it converges. For
max_standby_streaming_delay = -1, it anyway means "wait forever for
conflicts to clear", so the new behaviour should be acceptable in that
case.

The other way to avoid looping here is to stop the chain from being
extended: past the deadline, refuse new conflicting snapshot imports
on the standby (aka "publish the intended horizon and reject
conflicting imports"). I feel that is over engineering, instead, the
current proposed solution looks reasonable to me but if we still want
to avoid looping then probably we can attempt something like "reject
conflicting imports" in master.

--
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: Alexander Lakhin
Date:
Subject: Re: aio tests failing on newer Linux kernels
Next
From: Peter Smith
Date:
Subject: Missing list_free in publicationcmds.c:OpenTableList