On Mon, May 5, 2025 at 2:59 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Fri, May 2, 2025 at 3:05 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Fri, May 2, 2025 at 12:57 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > >
> > >
> > > Please find the v11 patch addressing the above points and all other
> > > comments. I have also optimized the test by reducing the number of
> > > subscriptions and slots.
> > >
> >
> > Thanks for the patch. Few comments:
> >
> > 1)
> >
> > pg_upgrade/t/003_logical_slots.pl:
> >
> > - "SELECT slot_name, two_phase, failover FROM pg_replication_slots");
> > -is($result, qq(regress_sub|t|t), 'check the slot exists on new cluster');
> > + "SELECT slot_name, two_phase FROM pg_replication_slots");
> > +is($result, qq(regress_sub|t), 'check the slot exists on new cluster');
> >
> > With this change, the failover property test during upgrade is
> > removed. Shall we add it again either using ALTER SUB to enable
> > failover and two_phase together or a new subscription with failover
> > alone?
> >
>
> If ALTER SUB is used to set failover for the existing subscription,
> then pg_upgrade will fail while trying to create a slot with both
> failover and two_phase on the upgraded node $newpub.
> So, I've implemented the other suggestion by adding a separate
> pub-subs to verify the failover property.
>
> > 2)
> > + The default is false. This option cannot be set together with
> > + <literal>failover</literal> when creating a logical replication slot.
> > + However, once the slot is created with <literal>two_phase =
> > true</literal>,
> > + <literal>failover</literal> can be set to true after the slot has
> > + consumed all transactions up to the point where two-phase decoding
> > + was enabled.
> >
> >
> > Suggestion: all transactions --> all the changes.
> >
>
> Done.
> ~~~
>
> Please find the v12 patch with above suggested changes.
Thank you for updating the patch! I'll review it and share comments.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
