Re: Fix slot synchronization with two_phase decoding enabled

On Thu, Apr 24, 2025 at 3:57 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Apr 24, 2025 at 2:54 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > Please find the updated patch for Approach 3, which implements the
> > idea suggested by Swada-san above.
> >
>
> Thank You for the patch.
>
> 1)
>
> CreateDecodingContext:
>
> if (ctx->twophase && !slot->data.two_phase)
> {
> + /*
> + * Do not allow two-phase decoding for failover enabled slots.
> + *
> + * See comments atop the similar check in ReplicationSlotCreate() for
> + * a detailed reason.
> + */
> + if (slot->data.failover)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot enable two-phase decoding for failover enabled slot \"%s\"",
> + NameStr(slot->data.name))));
> +
>
> Can you please add tetscase to cover this scenario?
>

Added a test for the above scenario.

> 2)
> We shall update create-sub documents as well for these mutually
> exclusive options. Review other pages (alter-sub, create-slot) as well
> for any required change.
>

Updated docs where I felt this new change should be mentioned. Please
let me know if I missed any place.

> 3)
> +##################################################
> +# Test that the failover option can be enabled for a two_phase enabled
> +# subscription.
> +##################################################
>
> Suggestion: 'Test that the failover option can be enabled for a two_phase
> enabled subscription only through Alter Subscription (failover=true)'
>
>

Done.
~~~

Please find the attached v8 patch with above comments addressed.
This version includes the documentation updates suggested by
Sawada-san at [1], and incorporates his feedback from [2] as well.

[1] https://www.postgresql.org/message-id/CAD21AoD08Nb588fAJ%2BJd5xRxtAT8yWnOfZ3zq-K5sto9b3ntsA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAD21AoD08Nb588fAJ%2BJd5xRxtAT8yWnOfZ3zq-K5sto9b3ntsA%40mail.gmail.com

--
Thanks,
Nisha