Re: Single transaction in the tablesync worker?

On Sun, Jan 24, 2021 at 5:54 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > 4.
> > - /*
> > - * To build a slot name for the sync work, we are limited to NAMEDATALEN -
> > - * 1 characters.  We cut the original slot name to NAMEDATALEN - 28 chars
> > - * and append _%u_sync_%u (1 + 10 + 6 + 10 + '\0').  (It's actually the
> > - * NAMEDATALEN on the remote that matters, but this scheme will also work
> > - * reasonably if that is different.)
> > - */
> > - StaticAssertStmt(NAMEDATALEN >= 32, "NAMEDATALEN too small"); /* for sanity */
> > - slotname = psprintf("%.*s_%u_sync_%u",
> > - NAMEDATALEN - 28,
> > - MySubscription->slotname,
> > - MySubscription->oid,
> > - MyLogicalRepWorker->relid);
> > + /* Calculate the name of the tablesync slot. */
> > + slotname = ReplicationSlotNameForTablesync(
> > +    MySubscription->oid,
> > +    MyLogicalRepWorker->relid);
> >
> > What is the reason for changing the slot name calculation? If there is
> > any particular reasons, then we can add a comment to indicate why we
> > can't include the subscription's slotname in this calculation.
> >
>
> The subscription slot name may be changed (e.g. ALTER SUBSCRIPTION)
> and so including the subscription slot name as part of the tablesync
> slot name was considered to be:
> a) possibly risky/undefined, if the subscription slot_name = NONE
> b) confusing, if we end up using 2 different slot names for the same
> tablesync (e.g. if the subscription slot name is changed before a sync
> worker is re-launched).
> And since this subscription slot name part is not necessary for
> uniqueness anyway, it was removed from the tablesync slot name to
> eliminate those concerns.
>
> Also, the tablesync slot name calculation was encapsulated as a
> separate function because previously (i.e. before v18) it was used by
> various other cleanup codes. I still like it better as a function, but
> now it is only called from one place so we could put that code back
> inline if you prefer it how it was..

It turns out those (a/b) concerns I wrote above are maybe unfounded,
because it seems not possible to alter the slot_name = NONE unless the
subscription is first DISABLED.
So probably I can revert all this tablesync slot name calculation back
to how it originally was in the OSS HEAD if you want.

----
Kind Regards,
Peter Smith.
Fujitsu Australia