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? 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. thanks Shveta
