Horton
Menu

Re: Implement waiting for wal lsn replay: reloaded

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Xuneng Zhou <xunengzhou@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Heikki Linnakangas <hlinnaka@iki.fi>, Peter Eisentraut <peter@eisentraut.org>, Thomas Munro <thomas.munro@gmail.com>, Álvaro Herrera <alvherre@kurilemu.de>, Chao Li <li.evan.chao@gmail.com>, pgsql-hackers <pgsql-hackers@lists.postgresql.org>, Michael Paquier <michael@paquier.xyz>, jian he <jian.universality@gmail.com>, Tomas Vondra <tomas@vondra.me>, Yura Sokolov <y.sokolov@postgrespro.ru>
Date: 2026-04-20T18:45:51Z
Lists: pgsql-hackers

Attachments

On Fri, Apr 10, 2026 at 9:13 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> On Fri, Apr 10, 2026 at 11:59 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2026 at 12:18 AM Andres Freund <andres@anarazel.de> wrote:
> > >
> > > On 2026-04-09 18:21:24 +0300, Alexander Korotkov wrote:
> > > > I've assembled all the pending patches together.
> > > > 0001 adds memory barrier to GetWalRcvWriteRecPtr() as suggested by
> > > > Andres off-list.
> > >
> > > I'd make it a pg_atomic_read_membarrier_u64().
> > >
> > >
> > > > 0002 is basically [1] by Xuneng, but revised given we have a memory
> > > > barrier in 0001, and my proposal to do ResetLatch() unconditionally
> > > > similar to our other Latch-based loops.
> > > > 0003 and 0004 are [2] by Xuneng.
> > > > 0005 is [3] by Xuneng.
> > > >
> > > > I'm going to add them to Commitfest to run CI over them, and have a
> > > > closer look over them tomorrow.
> > >
> > > Briefly skimming the patches, none makes the writes to writtenUpto use
> > > something bearing barrier semantics. I'd just make both of them a
> > > pg_atomic_write_membarrier_u64().
> > >
> >
> > Makes sense to me. Done.
> >
> > > I think this also needs a few more tests, e.g. for the scenario that
> > > 29e7dbf5e4d fixed.  I think it'd also be good to do some testing for
> > > off-by-one dangers. E.g. making sure that we don't stop waiting too early /
> > > too late.  Another one that I think might deserve more testing is waits on the
> > > standby while crossing timeline boundaries.
> > >
> >
> > I'll prepare a new patch for more test harnessing.
> >
> > >
> > > > From 0e5b4d1b9311a628a70218d89abf12308c9d782f Mon Sep 17 00:00:00 2001
> > > > From: Alexander Korotkov <akorotkov@postgresql.org>
> > > > Date: Thu, 9 Apr 2026 16:49:04 +0300
> > > > Subject: [PATCH v3 1/5] Add a memory barrier to GetWalRcvWriteRecPtr()
> > > >
> > > > Add pg_memory_barrier() before reading writtenUpto so that callers see
> > > > up-to-date shared memory state.  This matches the barrier semantics that
> > > > GetWalRcvFlushRecPtr() and other LSN-position functions get implicitly from
> > > > their spinlock acquire/release, and in turn protects from bugs caused by
> > > > expectations of similar barrier guarantees from different LSN-position functions.
> > > >
> > > > Reported-by: Andres Freund <andres@anarazel.de>
> > > > Discussion: https://postgr.es/m/zqbppucpmkeqecfy4s5kscnru4tbk6khp3ozqz6ad2zijz354k%40w4bdf4z3wqoz
> > > > ---
> > > >  src/backend/replication/walreceiverfuncs.c | 12 ++++++++++--
> > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
> > > > index bd5d47be964..0408ddff43e 100644
> > > > --- a/src/backend/replication/walreceiverfuncs.c
> > > > +++ b/src/backend/replication/walreceiverfuncs.c
> > > > @@ -363,14 +363,22 @@ GetWalRcvFlushRecPtr(XLogRecPtr *latestChunkStart, TimeLineID *receiveTLI)
> > > >
> > > >  /*
> > > >   * Returns the last+1 byte position that walreceiver has written.
> > > > - * This returns a recently written value without taking a lock.
> > > > + *
> > > > + * Use a memory barrier to ensure that callers see up-to-date shared memory
> > > > + * state, matching the barrier semantics provided by the spinlock in
> > > > + * GetWalRcvFlushRecPtr() and other LSN-position functions.
> > > >   */
> > > >  XLogRecPtr
> > > >  GetWalRcvWriteRecPtr(void)
> > > >  {
> > > >       WalRcvData *walrcv = WalRcv;
> > > > +     XLogRecPtr      recptr;
> > > > +
> > > > +     pg_memory_barrier();
> > > >
> > > > -     return pg_atomic_read_u64(&walrcv->writtenUpto);
> > > > +     recptr = pg_atomic_read_u64(&walrcv->writtenUpto);
> > > > +
> > > > +     return recptr;
> > > >  }
> > > >
> > > >  /*
> > > > --
> > > > 2.39.5 (Apple Git-154)
> > > >
> > >
> > > > Subject: [PATCH v3 2/5] Fix memory ordering in WAIT FOR LSN wakeup mechanism
> > >
> > > > +     /*
> > > > +      * Ensure the waker's prior position store (writtenUpto, flushedUpto,
> > > > +      * lastReplayedEndRecPtr, etc.) is globally visible before we read
> > > > +      * minWaitedLSN.  Without this barrier, the CPU could load minWaitedLSN
> > > > +      * before draining the position store, leaving the position invisible to a
> > > > +      * concurrently-registering waiter.
> > > > +      *
> > > > +      * This is the waker side of a Dekker-style handshake; pairs with
> > > > +      * pg_memory_barrier() in GetCurrentLSNForWaitType() on the waiter side.
> > > > +      */
> > > > +     pg_memory_barrier();
> > > > +
> > > >       /*
> > > >        * Fast path check.  Skip if currentLSN is InvalidXLogRecPtr, which means
> > > >        * "wake all waiters" (e.g., during promotion when recovery ends).
> > >
> > > I'd also make this a pg_atomic_read_membarrier_u64() and the write a
> > > pg_atomic_write_membarrier_u64().  It's a lot easier to reason about this
> > > stuff if you make sure that the individual reads / write pair and have
> > > ordering implied.
> > >
> >
> > It does look more selft-contained to me.
> >
> > Here is the updated patch set based on Alexander’s earlier version.
>
> The last para of commit message in patch 2 is inaccurate after the
> getter-side barrier changes, "could read a stale position and wrongly
> timeout" is no longer the primary rationale.

The updated patchset is attached.  It includes improved coverage as
suggested by Andres upthread.  And documentation that WAIT FOR LSN is
timeline-blind (per off-list discussion with Xuneng).

------
Regards,
Alexander Korotkov
Supabase

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Clean up 019_replslot_limit.pl comments

  2. Stabilize 019_replslot_limit.pl: wait on slot restart_lsn

  3. Fix memory ordering in WAIT FOR LSN wakeup mechanism

  4. Improve WAIT FOR LSN test coverage

  5. Remove redundant WAIT FOR LSN caller-side pre-checks

  6. Use barrier semantics when reading/writing writtenUpto

  7. Use replay position as floor for WAIT FOR LSN standby_(write|flush)

  8. Wake standby_write/standby_flush waiters from the WAL replay loop

  9. Minimal fix for WAIT FOR ... MODE 'standby_flush'

  10. Avoid syscache lookup while building a WAIT FOR tuple descriptor

  11. Document that WAIT FOR may be interrupted by recovery conflicts

  12. Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()

  13. Wake LSN waiters before recovery target stop

  14. Remove redundant pg_unreachable() after elog(ERROR) from ExecWaitStmt()

  15. Revert "Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()"

  16. Fix variable usage in wakeupWaiters()

  17. Add tab completion for the WAIT FOR LSN MODE option

  18. Add the MODE option to the WAIT FOR LSN command

  19. Extend xlogwait infrastructure with write and flush wait types

  20. Unify error messages

  21. Optimize shared memory usage for WaitLSNProcInfo

  22. Fix WaitLSNWakeup() fast-path check for InvalidXLogRecPtr

  23. Fix incorrect function name in comments

  24. Add infrastructure for efficient LSN waiting

  25. Add pairingheap_initialize() for shared memory usage

  26. Implement WAIT FOR command