Re: Synchronizing slots from primary to standby

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