Re: Implement waiting for wal lsn replay: reloaded

Xuneng Zhou <xunengzhou@gmail.com>

From: Xuneng Zhou <xunengzhou@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Álvaro Herrera <alvherre@alvh.no-ip.org>, Tomas Vondra <tomas@vondra.me>, Yura Sokolov <y.sokolov@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Date: 2025-09-14T13:51:21Z
Lists: pgsql-hackers

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. Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()

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

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

  6. Extend xlogwait infrastructure with write and flush wait types

  7. Unify error messages

  8. Optimize shared memory usage for WaitLSNProcInfo

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

  10. Fix incorrect function name in comments

  11. Add infrastructure for efficient LSN waiting

  12. Add pairingheap_initialize() for shared memory usage

  13. Implement WAIT FOR command

Hi Alexander,

On Sun, Sep 14, 2025 at 3:31 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> Hi, Xuneng!
>
> On Wed, Aug 27, 2025 at 6:54 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > I did a rebase for the patch to v8 and incorporated a few changes:
> >
> > 1) Updated documentation, added new tests, and applied minor code
> > adjustments based on prior review comments.
> > 2) Tweaked the initialization of waitReplayLSNState so that
> > non-backend processes can call wait for replay.
> >
> > Started a new thread [1] and attached a patch addressing the polling
> > issue in the function
> > read_local_xlog_page_guts built on the infra of patch v8.
> >
> > [1] https://www.postgresql.org/message-id/CABPTF7Vr99gZ5GM_ZYbYnd9MMnoVW3pukBEviVoHKRvJW-dE3g@mail.gmail.com
> >
> > Feedbacks welcome.
>
> Thank you for your reviewing and revising this patch.
>
> I see you've integrated most of your points expressed in [1].  I went
> though them and I've integrated the rest of them.  Except this one.
>
> > 11) The synopsis might read more clearly as:
> > - WAIT FOR LSN '<lsn>' [ TIMEOUT <milliseconds | 'duration-with-units'> ] [ NO_THROW ]
>
> I didn't find examples on how we do the similar things on other places
> of docs.  This is why I decided to leave this place as it currently
> is.

+1. I re-check other commands with similar parameter patterns, and
they follow the approach in v9.

>
> Also, I found some mess up with typedefs.list.  I've returned the
> changes to typdefs.list back and re-indented the sources.

 Thanks for catching and fixing that.

> I'd like to ask your opinion of the way this feature is implemented in
> terms of grammar: generic parsing implemented in gram.y and the rest
> is done in wait.c.  I think this approach should minimize additional
> keywords and states for parsing code.  This comes at the price of more
> complex code in wait.c, but I think this is a fair price.

It's LGTM. The same pattern is observed in VACUUM, EXPLAIN, and CREATE
PUBLICATION - all use minimal grammar rules that produce generic
option lists, with the actual interpretation done in their respective
implementation files. The moderate complexity in wait.c seems
acceptable.

Best,
Xuneng