Re: Implement waiting for wal lsn replay: reloaded

On Tue, Jul 28, 2026 at 9:25 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> On Mon, Jul 27, 2026 at 7:03 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > Hi Alexander,
> >
> > On Thu, Jul 9, 2026 at 8:24 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> > >
> > > Hi!
> > >
> > > As Noah pointed out in [1], auxiliary processes are supported in the
> > > facility, but proper cleanup is absent when they exit. I have attached
> > > a patch trying to address that as suggested before getting sidetracked
> > > for too long with the issue raised in [2].
> >
> > I'd like to propose a series of patches to do some further clean-ups &
> > enhancements. The first few are mostly mechanical, straightforward,
> > and the last two are more subtle. I appreciate your inputs/thoughts on
> > them.
> >
> > 1) Patch 1 to address the issue raised by Noah.
> >
> > 2) Patch 2 to fix two small docs mismatches.
> >
> > 3) Patch 3 to optimize the WaitLSNLock acquisition in deleteLSNWaiter.
> >
> > 4) Patch 4 to clarify lsn waiter cleanup after wakeup.
> >
> > 5) Patch 5 to address a rare issue of deregistration of waiters whose
> > target lsn has reached but then flashed back.
> >
> > Currently, the waiters are removed from the heap by the wakers if
> > their target lsn has reached. We offer no re-registration service in
> > the reception desk for them assuming they won't need to extend their
> > stay. This assumption holds if the lsn is monotonically increasing,
> > which is true for most cases. However, there seems to be a catch for
> > the wal write/flush lsn from the wal receiver side. In the cases of
> > restarting wal receiver, the write/flush lsn could be reset to
> > positions lagging behind(the attached restarting cases file summarized
> > by Sol with inputs from me shows a matrix of this).
> >
> > One problematic interleaving, for reset cases 2–4 in the attached
> > file, is the following:
> >
> > Let W be the old published write position, X the waiter’s target, S
> > the new streaming segment start, and R the replay position, with:
> > max(N, R) < X ≤ W
> >
> > [T0] A backend registers a standby_write waiter for target X and sleeps.
> >
> > [T1] The walreceiver publishes writtenUpto = W and calls
> > WaitLSNWakeup(W). Because X ≤ W, the waker removes the waiter. from
> > the heap, sets inHeap = false, and sets its latch.
> >
> > [T2] Before the waiter runs, streaming is restarted.
> > RequestXLogStreaming() resets shared writtenUpto from W to S. The
> > effective standby_write position is now: max(writtenUpto, replay) =
> > max(S, R) = R. Therefore the effective position has regressed below X.
> >
> > [T3] The waiter consumes the old latch notification and rechecks its
> > condition. It observes R < X. In the pre-fix code, it goes back to
> > sleep without re-registering, even though it is no longer in the
> > waiters heap.
>
> I took a second look at the wording. I had asked Sol to polish this
> part for clarity, but the revised version is inaccurate. It should be:
>
> [T2] Before the waiter runs, streaming is restarted.
> RequestXLogStreaming() resets writtenUpto from W to S. The effective
> standby_write position becomes max(S, R), which is below X.
>
> [T3] The waiter consumes the earlier latch notification and rechecks
> its condition. It observes that max(S, R) < X. In the pre-fix code, it
> waits again without re-registering, even though the waker has already
> removed it from the heap.
>
> Sorry for not double-checking it.
>
> > [T4] The walreceiver later receives and publishes WAL through X and
> > calls WaitLSNWakeup() again. The waiter is absent from the heap, so it
> > receives no notification and can remain asleep even though its target
> > has now been reached.
> >
> > One possible fix for the issue that I have considered is to make
> > write/flush lsn monotonic so it won't flash back once advanced. This
> > seems to work well at first glance. However, it has two shortcomings:
> > one is that it can blur the definition of write lsn and another is
> > that it won't fix the cross-timeline lsn regression. Currently,
> > writtenUpto is the current walreceiver stream’s write frontier. Giving
> > that value monotonicity would turn it into the greatest numeric lsn
> > ever written by any receiver incarnation. Cross-timeline lsn flashback
> > is expected given the fork point is smaller than the pre-restart lsns.
> > Extending it across timelines would incorrectly associate progress on
> > an old timeline with the new timeline. The patch places the fix in the
> > waiter side -- let the backend do a recheck and re-registration if the
> > published lsn is regressed.
> >
> > Another aspect is the test, it's relatively simple to construct the
> > failure case, but seems hard to do so deterministically. I spent a lot
> > of time(maybe too much for an unconfirmed rare issue) trying to tame
> > it in a simple way; it seems hard to orchestrate the startup process,
> > wal receiver, waiting backend into above-like interleavings end-to-end
> > without using complex synchronization like three injection points. The
> > tricky part is to ensure max(N, R) < X ≤ W. We cannot stop the startup
> > then the wal receiver to make sure that because we need the former to
> > manipulate the later one. The current workaround is to add a synthetic
> > helper for removing the heap node even if its target lsn has not
> > actually reached, then trigger the real wake-up call by advancing the
> > published lsn to check whether the waiter is notified and the wait
> > completes. The rationale behind this is that the premature removal
> > from the heap without re-registration is the underlying issue
> > regardless of the specific lsn types and failure scenarios. That said,
> > I am unsure whether this is a proper way or better alternatives exist.
> >
> > 6) Patch 6 to cover a missing wake-up point for primary-flush waiters [WIP]

After learning more of the test/modules, I became less convinced to
use an injection point for the proof of re-registration in the test
which seems somewhat hacky to me. It works in a non-obvious manner --
we infer from the position of the code that the registration occurs,
but it is prone to future changes unless we add more comments to
explain its usage. Just wondering whether it makes sense to add a
helper to probe & prove the waiter is registered in the heap.

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