Re: [HACKERS] logical decoding of two-phase transactions

On Wed, Jun 9, 2021 at 10:34 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Tue, Jun 8, 2021 at 4:19 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> > > 3.
> > > @@ -432,10 +432,19 @@ CreateInitDecodingContext(const char *plugin,
> > > MemoryContextSwitchTo(old_context);
> > >
> > > /*
> > > - * We allow decoding of prepared transactions iff the two_phase option is
> > > - * enabled at the time of slot creation.
> > > + * We allow decoding of prepared transactions when the two_phase is
> > > + * enabled at the time of slot creation, or when the two_phase option is
> > > + * given at the streaming start.
> > > */
> > > - ctx->twophase &= MyReplicationSlot->data.two_phase;
> > > + ctx->twophase &= (ctx->twophase_opt_given || slot->data.two_phase);
> > > +
> > > + /* Mark slot to allow two_phase decoding if not already marked */
> > > + if (ctx->twophase && !slot->data.two_phase)
> > > + {
> > > + slot->data.two_phase = true;
> > > + ReplicationSlotMarkDirty();
> > > + ReplicationSlotSave();
> > > + }
> > >
> > > Why do we need to change this during CreateInitDecodingContext which
> > > is called at create_slot time? At that time, we don't need to consider
> > > any options and there is no need to toggle slot's two_phase value.
> > >
> > >
> >
> > TODO
>
> As part of the recent changes, we do turn on two_phase at create_slot time when
> the subscription is created with (copy_data = false, two_phase = on).
> So, this code is required.
>

But in that case, won't we deal it with the value passed in
CreateReplicationSlotCmd. It should be enabled after we call
ReplicationSlotCreate.

--
With Regards,
Amit Kapila.