On Tuesday, November 21, 2023 5:33 PM shveta malik <shveta.ma...@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. Or we could just document that it is user's responsibility to match the failover property in case it changes the slot_name. Thoughts ? Best Regards, Hou zj
