Re: Implement waiting for wal lsn replay: reloaded
Andres Freund <andres@anarazel.de>
Hi, On 2026-04-06 23:07:45 -0400, Andres Freund wrote: > But, leaving that aside, looking at this code I'm somewhat concerned - it > seems to not worry at all about memory ordering? > > > static void > XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr, TimeLineID tli) > ... > /* Update shared-memory status */ > pg_atomic_write_u64(&WalRcv->writtenUpto, LogstreamResult.Write); > > /* > * If we wrote an LSN that someone was waiting for, notify the waiters. > */ > if (waitLSNState && > (LogstreamResult.Write >= > pg_atomic_read_u64(&waitLSNState->minWaitedLSN[WAIT_LSN_TYPE_STANDBY_WRITE]))) > WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_WRITE, LogstreamResult.Write); > > There are no memory barriers here, so the CPU would be entirely free to not > make the writtenUpto write visible to a waiter that's in the process of > registering and is checking whether it needs to wait in WaitForLSN(). > > And WaitForLSN()->GetCurrentLSNForWaitType()->GetWalRcvWriteRecPtr() also has > no barriers. That MAYBE is ok, due addLSNWaiter() providing the barrier at > loop entry and maybe kinda you can think that WaitLatch() will somehow also > have barrier semantic. But if so, that would need to be very carefully > documented. And it seems completely unnecessary here, it's hard to believe > using a barrier (via pg_atomic_read_membarrier_u64() or such) would be a > performance issue And separately from the memory ordering, how can it make sense that there's at least 5 copies of this if (waitLSNState && (LogstreamResult.Flush >= pg_atomic_read_u64(&waitLSNState->minWaitedLSN[WAIT_LSN_TYPE_STANDBY_FLUSH]))) WaitLSNWakeup(WAIT_LSN_TYPE_STANDBY_FLUSH, LogstreamResult.Flush); around? That needs to be encapsulated so that if you have a bug, like the memory ordering problem I describe above, it can be fixed once, not in multiple places. And why do these callers even have that pre-check? Seems WaitLSNWakeup() does so itself? /* * Fast path check. Skip if currentLSN is InvalidXLogRecPtr, which means * "wake all waiters" (e.g., during promotion when recovery ends). */ if (XLogRecPtrIsValid(currentLSN) && pg_atomic_read_u64(&waitLSNState->minWaitedLSN[i]) > currentLSN) return; And why is the code checking if waitLSNState is non-NULL? Greetings, Andres Freund
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