Re: Implement waiting for wal lsn replay: reloaded

On Fri, Jul 31, 2026 at 8:58 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> On Thu, Jul 30, 2026 at 10:51 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > 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.
> >
>
> Updated the patch 5 according to the methodology. Added a new test
> helper, test_wait_lsn_waiter_is_registered(), that checks whether a
> process is registered in the waiters heap for a specified target LSN,
> wait type, and PID.

Generalized the test_wait_lsn_wakeup helper to specify the wait type
to pair with test_wait_lsn_waiter_is_registered.

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