On Mon, Oct 23, 2023 at 11:22 PM Drouvot, Bertrand <bertranddrouvot...@gmail.com> wrote: > > Hi, > > On 10/20/23 5:27 AM, shveta malik wrote: > > On Wed, Oct 18, 2023 at 4:24 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > PFA v25 patch set. The changes are: > > > > 1) 'enable_failover' is changed to 'failover' > > 2) Alter subscription changes to support 'failover' > > 3) Fixes a bug in patch001 wherein any change in standby_slot_names > > was not considered in the flow where logical walsenders wait for > > standby's confirmation. Now during the wait, if standby_slot_names is > > changed, wait is restarted using new standby_slot_names. > > 4) Addresses comments by Bertrand and Amit in [1],[2],[3] > > > > The changes are mostly in patch001 and a very few in patch002. > > > > Thank You Ajin for working on alter-subscription changes and adding > > more TAP-tests for 'failover' > > > > Thanks for updating the patch! > > Looking at 0001 and doing some experiment: > > Creating a logical slot with failover = true and then launching > pg_logical_slot_get_changes() or pg_recvlogical() on it results > to setting failover back to false. > > It occurs while creating the decoding context here: > > @@ -602,6 +602,9 @@ CreateDecodingContext(XLogRecPtr start_lsn, > SnapBuildSetTwoPhaseAt(ctx->snapshot_builder, start_lsn); > } > > + /* set failover in the slot, as requested */ > + slot->data.failover = ctx->failover; > + > > I think we can get rid of this change in CreateDecodingContext(). > Yes, I too noticed this in my testing, however just removing this from CreateDecodingContext will not allow us to change the slot's failover flag using Alter subscription. Currently alter subscription re-establishes the connection using START REPLICATION and failover is one of the options passed in along with START REPLICATION. I am thinking of moving this change to StartLogicalReplication prior to calling CreateDecodingContext by parsing the command options in StartReplicationCmd without adding it to the LogicalDecodingContext.
regards, Ajin Cherian Fujitsu Australia
