Re: [HACKERS] make async slave to wait for lsn to be replayed

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>
To: Kevin Hale Boyes <kcboyes@gmail.com>
Cc: Kartyshov Ivan <i.kartyshov@postgrespro.ru>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, hlinnaka@iki.fi, alvherre@alvh.no-ip.org, pashkin.elfe@gmail.com, bharath.rupireddyforpostgres@gmail.com, euler@eulerto.com, thomas.munro@gmail.com, peter@eisentraut.org, amit.kapila16@gmail.com, dilipbalaut@gmail.com, smithpb2250@gmail.com, pgsql-hackers@lists.postgresql.org
Date: 2024-08-10T16:33:57Z
Lists: pgsql-hackers

Attachments

On Tue, Aug 6, 2024 at 5:17 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Sat, Aug 3, 2024 at 6:07 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Sat, Aug 3, 2024 at 3:45 AM Kevin Hale Boyes <kcboyes@gmail.com> wrote:
> > > In the for loop in WaitForLSNReplay, shouldn't the check for in-recovery be moved up above the call to GetXLogReplayRecPtr?
> > > If we get promoted while waiting for the timeout we could call GetXLogReplayRecPtr while not in recovery.
> >
> > This is intentional.  After standby gets promoted,
> > GetXLogReplayRecPtr() returns the last WAL position being replayed
> > while being standby.  So, if standby reached target lsn before being
> > promoted, we don't have to throw an error.
> >
> > But this gave me an idea that before the loop we probably need to put
> > RecoveryInProgress() check after GetXLogReplayRecPtr() too.  I'll
> > recheck that.
>
> The attached patchset comprises assorted improvements for pg_wal_replay_wait().
>
> The 0001 patch is intended to improve this situation.  Actually, it's
> not right to just put RecoveryInProgress() after
> GetXLogReplayRecPtr(), because more wal could be replayed between
> these calls.  Instead we need to recheck GetXLogReplayRecPtr() after
> getting negative result of RecoveryInProgress() because WAL replay
> position couldn't get updated after.
> 0002 patch comprises fix for the header comment of WaitLSNSetLatches() function
> 0003 patch comprises tests for pg_wal_replay_wait() errors.

Here is a revised version of the patchset.  I've fixed some typos,
identation, etc.  I'm going to push this once it passes cfbot.

------
Regards,
Alexander Korotkov
Supabase

Commits

  1. Ensure standby promotion point in 043_wal_replay_wait.pl

  2. Minor cleanup related to pg_wal_replay_wait() procedure

  3. Adjust pg_wal_replay_wait() procedure behavior on promoted standby

  4. pg_wal_replay_wait(): Fix typo in the doc

  5. Implement pg_wal_replay_wait() stored procedure

  6. Revert: Implement pg_wal_replay_wait() stored procedure

  7. Call WaitLSNCleanup() in AbortTransaction()

  8. Clarify what is protected by WaitLSNLock

  9. Use an LWLock instead of a spinlock in waitlsn.c

  10. Use the pairing heap instead of a flat array for LSN replay waiters

  11. Minor improvements for waitlsn.c

  12. Make the order of the header file includes consistent