Re: Synchronizing slots from primary to standby

On Fri, Sep 8, 2023 at 1:54 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Sep 7, 2023 at 8:29 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Fri, Aug 25, 2023 at 2:15 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > >
> > > Wait a minute ...
> > >
> > > From bac0fbef8b203c530e5117b0b7cfee13cfab78b9 Mon Sep 17 00:00:00 2001
> > > From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)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