Re: Fix slot synchronization with two_phase decoding enabled

On Fri, Jun 6, 2025 at 12:37 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> Attached v18 patch.
> - patch-001: modified error messages as suggested above.
> - patch-002: improved pg_dump docs as per Shveta's off-list suggestions.
>

Thanks for the patches. Please find few comments:

1)
+ * However, we allow this combination in binary upgrade mode, where
+ * pg_upgrade guarantees that all slot changes are consumed and no
+ * prepared transactions exist.
+ */

Can we please mention on which system pg_upgrade ensures that no
prepared txn exists. Is it at the source system or target system or
both? The similar comment is there at other place too, please update
that as well.

2)

+ * The failover option cannot be set togather with two_phase during
+ * CREATE SUBSCRIPTION. It can be enabled later using ALTER
+ * SUBSCRIPTION after the subscription is restored.
+ */

togather->together

3)
We have these 2 messages in slot.c and subscriptioncmds.c, we can have
both in similar way to maintain consistency across the patch. Since we
are using 'options' in the msg, thus one with %s looks better

+ errmsg("cannot enable both failover and two_phase options during
replication slot creation"));

+ errmsg("cannot enable both \"%s\" and \"%s\" options at \"CREATE
SUBSCRIPTION\"",
+ "failover", "two_phase"),

4)
Can we please update patch-message for patch002 similar to doc to make
it more clear.

thanks
Shveta