On Thu, Oct 29, 2026 at 8:44 PM Zhijie Hou <[email protected]> wrote: > > On Thu, Oct 29, 2026 at 06:41 PM Bertrand Drouvot wrote: > > > > On Thu, Oct 29, 2026 at 03:52:46PM +0530, vignesh C wrote: > >> There is the following documentation mentioning that slots must be > >> created with the same parameters of the subscription at [1]. > >> When altering the slot_name, the failover and two_phase property > >> values of the named slot may differ from the counterpart failover and > >> two_phase parameters specified in the subscription. When creating the > >> slot, ensure the slot properties failover and two_phase match their > >> counterpart parameters of the subscription. Otherwise, the slot on the > >> publisher may behave differently from what these subscription options > >> say: for example, the slot on the publisher could either be synced to > >> the standbys even when the subscription's failover option is disabled > >> or could be disabled for sync even when the subscription's failover > >> option is enabled. > >> > >> I'm not sure if this should be handled here as it is documented already. > > > > I agree about the documentation, but the patch introduces a regression: > > without > > the patch, two_phase is enabled on the replacement slot. With the patch, it > > remains > > disabled. > > Like Vignesh, I think the documentation already requires users to create > the replacement slot with matching properties when altering slot_name, > so we don't promise that setting two_phase = true will update the > replacement slot. > > > It might be worth fixing, as preserving 'e' only when slot_name is unchanged > > should be simple enough. > > I don't think handling that here would be simple. Resetting ENABLED to > PENDING when slot_name is changed would re-introduce the same bugs that > this patch aims to fix: prepared transactions from the previous slot may > still exist on the subscriber, and if the replacement slot already has > two_phase enabled on the publisher, prepared transactions could again be > streamed concurrently with table synchronization. Avoiding that would > require checking prepared transactions and the remote slot's status > during the change, which would complicate the fix. >
I agree that we don't need to preserve the old behavior as it will open up other bugs and we already document that such slot's properties should match subscription's options. But I feel we should add that change in behavior for new slot's in the commit message. Other than that the latest patch in this thread looks good to me. -- With Regards, Amit Kapila.
