On Mon, May 5, 2025 at 2:59 AM Nisha Moond <nisha.moond...@gmail.com> wrote: > > On Fri, May 2, 2025 at 3:05 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > On Fri, May 2, 2025 at 12:57 PM Nisha Moond <nisha.moond...@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
