Re: Fix slot synchronization with two_phase decoding enabled

On Fri, May 2, 2025 at 12:57 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)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