Horton

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: 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>, Álvaro Herrera <alvherre@kurilemu.de>
Date: 2025-10-23T10:46:27Z
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

Attachments

Hi!

In Thu, Oct 16, 2025 at 10:12 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> On Wed, Oct 15, 2025 at 8:48 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> >
> > Hi,
> >
> > Thank you for the grammar review and the clear recommendation.
> >
> > On Wed, Oct 15, 2025 at 4:51 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> > >
> > > I didn't review the patch other than look at the grammar, but I disagree
> > > with using opt_with in it.  I think WITH should be a mandatory word, or
> > > just not be there at all.  The current formulation lets you do one of:
> > >
> > > 1. WAIT FOR LSN '123/456' WITH (opt = val);
> > > 2. WAIT FOR LSN '123/456' (opt = val);
> > > 3. WAIT FOR LSN '123/456';
> > >
> > > and I don't see why you need two ways to specify an option list.
> >
> > I agree with this as unnecessary choices are confusing.
> >
> > >
> > > So one option is to remove opt_wait_with_clause and just use
> > > opt_utility_option_list, which would remove the WITH keyword from there
> > > (ie. only keep 2 and 3 from the above list).  But I think that's worse:
> > > just look at the REPACK grammar[1], where we have to have additional
> > > productions for the optional parenthesized option list.
> > >
> > >
> > >
> > > So why not do just
> > >
> > > +opt_wait_with_clause:
> > > +           WITH '(' utility_option_list ')'        { $$ = $3; }
> > > +           | /*EMPTY*/                             { $$ = NIL; }
> > > +           ;
> > >
> > > which keeps options 1 and 3 of the list above.
> >
> > Your suggested approach of making WITH mandatory when options are
> > present looks better.
> > I've implemented the change as you recommended. Please see patch 3 in v16.
> >
> > >
> > >
> > >
> > > Note: you don't need to worry about WITH_LA, because that's only going
> > > to show up when the user writes WITH TIME or WITH ORDINALITY (see
> > > parser.c), and that's a syntax error anyway.
> > >
> >
> > Yeah, we require '(' immediately after WITH in our grammar, the
> > lookahead mechanism will keep it as regular WITH, and any attempt to
> > write "WITH TIME" or "WITH ORDINALITY" would be a syntax error anyway,
> > which is expected.
> >
>
> The filename of patch 1 is incorrect due to coping. Just correct it.

Thank you for rebasing the patch.

I've revised it.  The significant changes has been made to 0002, where
I reduced the code duplication.  Also, I run pgindent and pgperltidy
and made other small improvements.
Please, check.

------
Regards,
Alexander Korotkov
Supabase