On Wed, Nov 22, 2023 at 10:02 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Tuesday, November 21, 2023 5:33 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> >
> > v37 fails to apply to HEAD due to a recent commit e83aa9f92fdd, rebased the
> > patches. PFA v37_2 patches.
>
> Thanks for updating the patches.
>
> I'd like to discuss one issue related to the correct handling of failover flag
> when executing ALTER SUBSCRIPTION SET (slot_name = 'new_slot')".
>
> Since the command intends to use a new slot on the primary, the new slot needs
> to reflect the "failover" state that the subscription currently has. If the
> failoverstate of the Subscription is LOGICALREP_FAILOVER_STATE_ENABLED, then I
> can reset it to LOGICALREP_FAILOVER_STATE_PENDING and allow the apply worker to
> handle it the way it is handled today (just like two_phase handling).
>
> But if the failoverstate is LOGICALREP_FAILOVER_STATE_DISABLED, the original
> idea is to call walrcv_alter_slot and alter the slot from the "ALTER
> SUBSCRIPTION" handling backend itself. This works if the slot is currently
> disabled. But the " ALTER SUBSCRIPTION SET (slot_name = 'new_slot')" command is
> supported even if the subscription is enabled. If the subscription is enabled,
> then calling walrcv_alter_slot() fails because the slot is still acquired by
> apply worker.
>
> So, I am thinking do we need a new mechanism to change the failover flag to
> false on an enabled subscription ? For example, we could call walrcv_alter_slot
> on startup of apply worker if AllTablesyncsReady(), for both true and false
> values of failover flag. This way, every time apply worker is started, it calls
> walrcv_alter_slot to set the failover flag on the primary.
>
I think for the false case, we need to execute walrcv_alter_slot()
every time at the start of apply worker and it doesn't sound like an
ideal way to achieve it.
> Or we could just document that it is user's responsibility to match the failover
> property in case it changes the slot_name.
>
Personally, I think we should document this behavior instead of
complicating the patch and the user anyway has a way to achieve it.
--
With Regards,
Amit Kapila.