Re: Synchronizing slots from primary to standby

On Fri, Sep 8, 2023 at 1:54 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Sep 7, 2023 at 8:29 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Fri, Aug 25, 2023 at 2:15 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > >
> > > Wait a minute ...
> > >
> > > From bac0fbef8b203c530e5117b0b7cfee13cfab78b9 Mon Sep 17 00:00:00 2001
> > > From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
> > > Date: Sat, 22 Jul 2023 10:17:48 +0000
> > > Subject: [PATCH v13 1/2] Allow logical walsenders to wait for physical
> > >  standbys
> > >
> > > @@ -2498,6 +2500,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
> > >                 }
> > >                 else
> > >                 {
> > > +                       /*
> > > +                        * Before we send out the last set of changes to logical decoding
> > > +                        * output plugin, wait for specified streaming replication standby
> > > +                        * servers (if any) to confirm receipt of WAL upto commit_lsn.
> > > +                        */
> > > +                       WaitForStandbyLSN(commit_lsn);
> > >
> > > OK, so we call this new function frequently enough -- once per
> > > transaction, if I read this correctly?  So ...
> > >
> > > +void
> > > +WaitForStandbyLSN(XLogRecPtr wait_for_lsn)
> > > +{
> > >  ...
> > >
> > > +       /* "*" means all logical walsenders should wait for physical standbys. */
> > > +       if (strcmp(synchronize_slot_names, "*") != 0)
> > > +       {
> > > +               bool    shouldwait = false;
> > > +
> > > +               rawname = pstrdup(synchronize_slot_names);
> > > +               SplitIdentifierString(rawname, ',', &elemlist);
> > > +
> > > +               foreach (l, elemlist)
> > > +               {
> > > +                       char *name = lfirst(l);
> > > +                       if (strcmp(name, NameStr(MyReplicationSlot->data.name)) == 0)
> > > +                       {
> > > +                               shouldwait = true;
> > > +                               break;
> > > +                       }
> > > +               }
> > > +
> > > +               pfree(rawname);
> > > +               rawname = NULL;
> > > +               list_free(elemlist);
> > > +               elemlist = NIL;
> > > +
> > > +               if (!shouldwait)
> > > +                       return;
> > > +       }
> > > +
> > > +       rawname = pstrdup(standby_slot_names);
> > > +       SplitIdentifierString(rawname, ',', &elemlist);
> > >
> > > ... do we really want to be doing the GUC string parsing every time
> > > through it?  This sounds like it could be a bottleneck, or at least slow
> > > things down.  Maybe we should think about caching this somehow.
> > >
> >
> > Yes, these parsed lists are now cached. Please see v15
> > (https://www.postgresql.org/message-id/CAJpy0uAuzbzvcjpnzFTiWuDBctnH-SDZC6AZabPX65x9GWBrjQ%40mail.gmail.com)
> >
> > thanks
> > Shveta
>
> Patches (v15) were no longer applying to HEAD, rebased those and
> addressed below along-with:
>
> 1) Fixed an issue in slots-invalidation code-path on standby. Thanks
> Ajin for testing the patch and finding the issue.
> 2) Ensure that WAL is replayed on standby before moving the slot's
> position to the target location received from the primary.
> 3) Some code restructuring in slotsync.c
>
> thanks
> Shveta

There were cfbot failures on v16 patches:
--presence of 051_slot_sync.pl in meson.build even though the file is removed.
--usage of uint in launcher.c

Fixed above and attached v16_2_0001/0002 patches again.

thanks
Shveta