On Fri, Jul 31, 2026 at 2:23 PM Xuneng Zhou <[email protected]> wrote: > > On Fri, Jul 31, 2026 at 1:02 PM Xuneng Zhou <[email protected]> wrote: > > > > On Fri, Jul 31, 2026 at 11:39 AM Xuneng Zhou <[email protected]> wrote: > > > > > > On Fri, Jul 31, 2026 at 9:46 AM Xuneng Zhou <[email protected]> wrote: > > > > > > > > On Fri, Jul 31, 2026 at 8:58 AM Xuneng Zhou <[email protected]> > > > > wrote: > > > > > > > > > > On Thu, Jul 30, 2026 at 10:51 AM Xuneng Zhou <[email protected]> > > > > > wrote: > > > > > > > > > > > > On Tue, Jul 28, 2026 at 9:25 PM Xuneng Zhou <[email protected]> > > > > > > wrote: > > > > > > > > > > > > > > On Mon, Jul 27, 2026 at 7:03 PM Xuneng Zhou > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > Hi Alexander, > > > > > > > > > > > > > > > > On Thu, Jul 9, 2026 at 8:24 PM Xuneng Zhou > > > > > > > > <[email protected]> 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. > > > > > > V4 improved some commit messages and comments of patch 1-4. > > > > There are two inaccurate expressions in the comments of patch 1 and 4. > > They are now fixed in v5. Sorry for the noise here. > > Here's v6 to update the comments just like the two immediate > precedents did. It seems annoying to do/say so. Should've shipped a > self-settled version. Sorry for haven't done otherwise...
The patchset looks good to me. I made some minor corrections. 0003 Removed extra check in WaitLSNCleanup() as deleteLSNWaiter() already does it. 0005 Improved comment about the situation on when LSN can go backwards. I'm going to push this if no objections. ------ Regards, Alexander Korotkov Supabase
v7-0003-Avoid-locking-when-an-LSN-waiter-is-already-remov.patch
Description: Binary data
v7-0004-Clarify-LSN-waiter-cleanup-after-wakeup.patch
Description: Binary data
v7-0002-Fix-WAIT-FOR-LSN-documentation-examples.patch
Description: Binary data
v7-0005-Re-register-LSN-waiters-after-stale-wakeups.patch
Description: Binary data
v7-0001-Add-wait-for-lsn-process-exit-cleanup-callback.patch
Description: Binary data
