Horton

Re: Implement waiting for wal lsn replay: reloaded

Xuneng Zhou <xunengzhou@gmail.com>

View thread
From: Xuneng Zhou <xunengzhou@gmail.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Tomas Vondra <tomas@vondra.me>, Yura Sokolov <y.sokolov@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Date: 2025-09-26T11:22:42Z
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 Álvaro,

Thanks for your review.

On Tue, Sep 16, 2025 at 4:24 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Sep-15, Alexander Korotkov wrote:
>
> > > 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.
>
> Actually I find the code in ExecWaitStmt pretty unusual.  We tend to use
> lists of DefElem (a name optionally followed by a value) instead of
> individual scattered elements that must later be matched up.  Why not
> use utility_option_list instead and then loop on the list of DefElems?
> It'd be a lot simpler.

I took a look at commands like VACUUM and EXPLAIN and they do follow
this pattern. v11 will make use of utility_option_list.

> Also, we've found that failing to surround the options by parens leads
> to pain down the road, so maybe add that.  Given that the LSN seems to
> be mandatory, maybe make it something like
>
> WAIT FOR LSN 'xy/zzy' [ WITH ( utility_option_list ) ]
>
> This requires that you make LSN a keyword, albeit unreserved.  Or you
> could make it
> WAIT FOR Ident [the rest]
> and then ensure in C that the identifier matches the word LSN, such as
> we do for "permissive" and "restrictive" in
> RowSecurityDefaultPermissive.

Shall make LSN an unreserved keyword as well.

Best,
Xuneng