Re: Implement waiting for wal lsn replay: reloaded

On Sun, Sep 6, 2026 at 10:35 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> On Wed, Aug 26, 2026 at 1:31 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > Hi Alexander,
> >
> > Thanks for taking care of the above patches.
> >
> > Here are four more to go. Your thoughts are appreciated. Sorry for
> > posting them late -- I underestimated the subtlety of them and the
> > time required to dispel some portion of that subtlety, plus being
> > sidetracked from thread to thread from time to time in the Odyssey of
> > issue reporting.
> >
> > [Alert] To accelerate the pace of bug fixing in this phase, some of
> > the writing below is co-authored with Sol. I remain responsible for
> > eliminating its hallucination and mine.
> >
> > 1) An unwanted survival after the hard-fought battle against deadlock
> >
> > [Disclosure] Sol did the first round investigation of a two-cycle
> > deadlock caused by holding relation lock, but it failed to generalize
> > the problem to three-cycles, rejected my v19 fix proposal which I
> > disagreed with and proposed several fixes which were turned down by
> > me. I took the helm for most of the analysis.
> >
> > ----- Prologue
> >
> > Waiting from too long to indefinite is what the command tried
> > relentlessly to avoid. To achieve this, lots of trade-offs &
> > compromises have been made regarding the snapshot management, let
> > alone the interface has been metamorphosed several times. However,
> > there seems to be an unwanted survival after the hard-fought battle.
> > Waiting in standby_replay, aka the default mode, can form a deadlock
> > with the startup process when executed by a transaction that retains
> > locks from earlier statements at READ COMMITTED. Unfortunately, this
> > deadlock could be permanent in certain scenarios.
> >
> > ----- Direct cycle
> >
> > Consider a standby backend B:
> > BEGIN;
> > SELECT * FROM tb;
> > WAIT FOR LSN '<future-lsn>' WITH (MODE 'standby_replay'); / WAIT FOR
> > LSN '<future-lsn>';
> >
> > The SELECT snapshot is released at statement end, so the snapshot
> > check permits the subsequent WAIT FOR. However, its AccessShareLock on
> > tb remains held until transaction end. If WAL below the target LSN
> > contains a DDL operation requiring recovery to acquire
> > AccessExclusiveLock on tb, such as ALTER TABLE or DROP TABLE, the
> > dependencies become:
> >
> > B waits for startup S to advance replay
> > S waits for B to release AccessShareLock(tb)
> >
> > B -> S -> B
> >
> > Both processes are then waiting for progress that only the other can provide.
> >
> > -- Why the deadlock is not detected
> > The startup process's heavyweight-lock wait is represented normally:
> >
> > S -> B
> >
> > Backend B's replay dependency is not represented in the
> > heavyweight-lock graph. WaitForLSN() sleeps on the backend latch,
> > rather than through ProcSleep():
> >
> > GetAwaitedLock() == NULL
> > B is not attached to a heavyweight-lock wait queue
> >
> > When startup's recovery deadlock timeout expires, it sends B a
> > RECOVERY_CONFLICT_STARTUP_DEADLOCK request. The current handler
> > contains the assumption that a backend not waiting for a heavyweight
> > lock cannot be deadlocked:
> >
> > if (GetAwaitedLock() == NULL)
> > return;
> >
> > Consequently, B ignores the request.
> > The actual and represented graphs differ as follows:
> >
> > Actual graph: B -> S -> B
> > Represented graph: S -> B
> > Missing dependency: B -> S
> >
> > This is not a lost-wakeup race. Both processes are correctly asleep,
> > but the dependency connecting the LSN-wait subsystem to the lock
> > manager is absent from deadlock detection.
> > The problem occurs in either ordering:
> > 1. B begins waiting first, after which startup blocks and probes B; B
> > ignores the probe.
> > 2. Startup blocks and completes its probe first, after which B begins
> > waiting; startup does not guarantee another probe.
> >
> > ----- Permanent behavior with unlimited standby delay
> >
> > With a finite max_standby_streaming_delay or
> > max_standby_archive_delay, the standby deadline eventually resolves
> > the situation as an ordinary recovery conflict. The waiting
> > transaction is canceled, its locks are released, and replay resumes.
> > With the relevant standby delay set to -1, however, there is no such
> > deadline. GetStandbyLimitTime() represents this as an unlimited wait.
> > After its deadlock probe, startup can enter an untimed second wait for
> > the relation lock. If the WAIT FOR command also has no timeout, the
> > cycle has no autonomous breaker:
> >
> > B cannot finish until startup replays
> > startup cannot replay until B finishes
> >
> > The result is an indefinite replay stall requiring external
> > intervention, such as canceling or terminating the backend, ending its
> > transaction, or promoting the standby.
> >
> > ------ Indirect cycle
> >
> > If the direct two-process cycle were the whole problem, the fix would
> > be much simpler. However,
> >
> > The missing dependency also permits longer cycles. For example:
> > B holds advisory lock L and waits for replay
> > C holds AccessShareLock(tb) and waits for L
> > S waits for AccessExclusiveLock(tb)
> >
> > The actual graph is:
> > B -> S -> C -> B
> > The heavyweight detector can represent:
> > S -> C -> B
> >
> > but traversal stops when it reaches B because B is sleeping in
> > WaitForLSN() rather than waiting for a heavyweight lock. This
> > demonstrates that the issue is not limited to the replay waiter
> > directly holding startup’s relation lock. Apart from the advisory
> > lock, can other heavy weight locks participate in the problematic
> > three-cycle?
> >
> > Here is the current-core assessment:
> >
> > On a hot standby, LockAcquireExtended() refuses any relation or object
> > lock stronger than RowExclusiveLock, and every mode conflicting with
> > AccessShareLock, RowShareLock, or RowExclusiveLock is itself stronger
> > than that. Two ordinary backends therefore cannot conflict on a
> > relation or object lock; only the startup process, which bypasses the
> > check, can hold AccessExclusiveLock. LOCK TABLE is classified to
> > match.
> >
> > The remaining classes fail on the holder side. A standby backend never
> > obtains an XID, so it cannot hold a transaction-ID lock. Tuple, page,
> > and speculative-token locks are taken only on write paths, as is
> > relation extension — which is excluded from cycle detection outright
> > in any case. A backend does hold its own VXID lock, but
> > VirtualXactLock() has exactly three callers: WaitForLockersMultiple()
> > and WaitForOlderSnapshots(), both DDL-only, and the startup process's
> > own non-blocking poll.
> >
> > That leaves advisory locks as the only core construction.
> > Extension-defined locktags remain open-ended, since the recovery
> > restriction covers only LOCKTAG_RELATION and LOCKTAG_OBJECT.
> >
> > ------ My proposal for v19
> >
> > Add a conservative fail-fast rule: before standby_replay wait, reject
> > it if the backend owns any granted heavyweight lock recorded in
> > 'LockMethodLocalHash' ('locallock->nLocks > 0').
> >
> > Although only relation- and advisory-lock cycles are the main concerns
> > here, limiting the check to those lock types would encode assumptions
> > about which core, extension, or future paths can wait on other lock
> > classes. Any locally represented heavyweight lock could become the
> > final edge back to the replay waiter. Scanning all granted 'LOCALLOCK'
> > entries seems simpler, more robust, and avoids maintaining a fragile
> > lock-type whitelist. The backend’s implicit VXID is not included
> > because it is not recorded in 'LockMethodLocalHash' and including it
> > would reject every transaction. No ordinary core hot-standby SQL
> > construction for C -> B through B's VXID has been shown. Current uses
> > of WAIT FOR in tap tests are unaffected by the new proposal per
> > inspection by Sol. All local tests passed.
>
> Sadly, we might need to extend this guard to flush/write waiters as
> well since the deadlock could form in archive recovery mode.
>
> standby_replay = replay position
> standby_write = max(receiver write position, replay position)
> standby_flush = max(receiver flush position, replay position)
>
> This ceiling makes write/flush progress implicitly depend on the replay.
>
> We could somehow relax the restriction for waiters in streaming mode.
> I don't know whether it is a good time to do so or the complexity is
> worthwhile.

Attached is a reproducer for the described scenario.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.