On Mon, Oct 23, 2023 at 11:22 PM Drouvot, Bertrand
<bertranddrouvot.pg@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.kapila16@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