Horton
Menu

Re: Implement waiting for wal lsn replay: reloaded

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Xuneng Zhou <xunengzhou@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, 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-07-07T15:25:28Z
Lists: pgsql-hackers 
Hi, Heikki!

On Mon, Jul 6, 2026 at 2:04 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> 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?

Thank you for bringing this up.

+1 for moving calls into ApplyWalRecord()

Regarding the overhead.  I've tried to apply 158.6 MB of pgbench
generated WAL on my local Macbook M3 Pro.

 * With calls, sec: 1.95/1.86/1.91/1.85/1.90
 * Without calls, sec:  1.83/1.87/1.96/1.90/1.85

So, the difference is less than the measurement error.

Do you prefer to commit the movement of the calls yourself, or do you
prefer me to do it?

------
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