Re: Implement waiting for wal lsn replay: reloaded
Heikki Linnakangas <hlinnaka@iki.fi>
On 01/05/2026 05:44, Xuneng Zhou wrote: > On Wed, Apr 29, 2026 at 5:01 AM Alexander Korotkov <aekorotkov@gmail.com > <mailto:aekorotkov@gmail.com>> wrote: > >> LGTM, I've added some comments for new functions in 0006. I propose >> to push this patchset. Probably something is still missing and we >> will have to go back to this. But it seems to make a lot of aspects >> much better. > > I reviewed the patchset and found a potential issue in the test for > patch 5, similar to the log-checking problem in the cascading timeline- > switch test. I've applied a minor fix to address it. Other parts LGTM. I happened to look around this code now. To recap, the code in the main WAL redo loop now looks like this: > > /* > * Apply the record > */ > ApplyWalRecord(xlogreader, record, &replayTLI); > > /* > * Wake up processes waiting for standby replay, write, or flush > * LSN to reach current replay position. Replay implies that the > * WAL was already written and flushed to disk, so write and flush > * waiters can be woken at the replay position too. > */ > WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_REPLAY, > XLogRecoveryCtl->lastReplayedEndRecPtr); > WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_WRITE, > XLogRecoveryCtl->lastReplayedEndRecPtr); > WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_FLUSH, > XLogRecoveryCtl->lastReplayedEndRecPtr); That's not wrong, but I've got some comments: 1. It's reading XLogRecoveryCtl->lastReplayedEndRecPtr without a lock or atomics. That's ok, no other process modifies lastReplayedEndRecPtr, but it feels a little dirty. 2. We're now doing three extra function calls on every WAL record. This is a very hot path, and most of the time, we'll just take the fast path in WaitLSNWakeup to return without doing anything. Andres and others assumed up-thread that it's negligible (we used to have pre-checks here in the caller), but I wonder if you did any performance testing? 3. There are other "wakeup" calls inside ApplyWalRecord(), to wake up walsenders and walreceivers. They could perhaps use the same wait-lsn machinery now, but that's v20 material. However, I think these WaitLSNWakeup() calls should also be moved inside ApplyWalRecord(), so that we'd have all the wakeup actions in one place. 4. Once you move those calls inside ApplyWalRecord(), like this: > @@ -1979,20 +1979,30 @@ ApplyWalRecord(XLogReaderState *xlogreader, XLogRecord *record, TimeLineID *repl > /* > * Update lastReplayedEndRecPtr after this record has been successfully > * replayed. > */ > SpinLockAcquire(&XLogRecoveryCtl->info_lck); > XLogRecoveryCtl->lastReplayedReadRecPtr = xlogreader->ReadRecPtr; > XLogRecoveryCtl->lastReplayedEndRecPtr = xlogreader->EndRecPtr; > XLogRecoveryCtl->lastReplayedTLI = *replayTLI; > SpinLockRelease(&XLogRecoveryCtl->info_lck); > > + /* > + * Wake up processes waiting for standby replay, write, or flush LSN to > + * reach current replay position. Replay implies that the WAL was already > + * written and flushed to disk, so write and flush waiters can be woken at > + * the replay position too. > + */ > + WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_REPLAY, xlogreader->EndRecPtr); > + WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_WRITE, xlogreader->EndRecPtr); > + WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_FLUSH, xlogreader->EndRecPtr); > + > /* ------ > * Wakeup walsenders: > * > * On the standby, the WAL is flushed first (which will only wake up > * physical walsenders) and then applied, which will only wake up logical > * walsenders. It becomes clear that you don't actually need the memory barrier inside WaitLSNWakeup(). Not sure if they're needed for other callers, but here we have just released a spinlock, which acts as a memory barrier. It might not be worth relaxing, but it does seem a little silly. If nothing else, I'd like to move those calls into ApplyWalRecord() for clarity (point 3 above). What do you think? - Heikki
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Clean up 019_replslot_limit.pl comments
- 0b866bb90368 19 (unreleased) landed
-
Stabilize 019_replslot_limit.pl: wait on slot restart_lsn
- cdb1d1cf1de6 19 (unreleased) landed
-
Fix memory ordering in WAIT FOR LSN wakeup mechanism
- a80a593ab636 19 (unreleased) landed
-
Improve WAIT FOR LSN test coverage
- cb096e6d6981 19 (unreleased) landed
-
Remove redundant WAIT FOR LSN caller-side pre-checks
- df9f938ca2ec 19 (unreleased) landed
-
Use barrier semantics when reading/writing writtenUpto
- dfb690dd5237 19 (unreleased) landed
-
Use replay position as floor for WAIT FOR LSN standby_(write|flush)
- cba67b5b87f9 19 (unreleased) landed
-
Wake standby_write/standby_flush waiters from the WAL replay loop
- e7cd592174d9 19 (unreleased) landed
-
Minimal fix for WAIT FOR ... MODE 'standby_flush'
- 29e7dbf5e4da 19 (unreleased) landed
-
Avoid syscache lookup while building a WAIT FOR tuple descriptor
- 834038c1f8d5 19 (unreleased) landed
-
Document that WAIT FOR may be interrupted by recovery conflicts
- 10484c2cc75b 19 (unreleased) landed
-
Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()
- 7e8aeb9e483d 19 (unreleased) landed
- f30848cb05d4 19 (unreleased) landed
-
Wake LSN waiters before recovery target stop
- 20a8f783e15c 19 (unreleased) landed
-
Remove redundant pg_unreachable() after elog(ERROR) from ExecWaitStmt()
- 30776ca46865 19 (unreleased) landed
-
Revert "Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()"
- e54ce0b2da62 19 (unreleased) landed
-
Fix variable usage in wakeupWaiters()
- bf308639bfcf 19 (unreleased) landed
-
Add tab completion for the WAIT FOR LSN MODE option
- 76948337f724 19 (unreleased) landed
-
Add the MODE option to the WAIT FOR LSN command
- 49a181b5d634 19 (unreleased) landed
-
Extend xlogwait infrastructure with write and flush wait types
- 7a39f43d885b 19 (unreleased) landed
-
Unify error messages
- 502e256f2262 19 (unreleased) cited
-
Optimize shared memory usage for WaitLSNProcInfo
- 75e82b2f5a6f 19 (unreleased) landed
-
Fix WaitLSNWakeup() fast-path check for InvalidXLogRecPtr
- ede6acef4967 19 (unreleased) landed
-
Fix incorrect function name in comments
- 23792d738158 19 (unreleased) landed
-
Add infrastructure for efficient LSN waiting
- 3b4e53a075ea 19 (unreleased) landed
-
Add pairingheap_initialize() for shared memory usage
- 8af3ae0d4b36 19 (unreleased) landed
-
Implement WAIT FOR command
- 447aae13b030 19 (unreleased) landed