RE: Synchronizing slots from primary to standby

On Friday, October 20, 2023 11:27 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> 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. Here are few things I noticed when testing the patch.
1)
+++ b/src/backend/replication/logical/logical.c
@@ -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 noticed others also commented this change. I found this will over-write the
slot's failover value to a wrong one in the case when we have acquired the slot and
call CreateDecodingContext after that. (e.g. it can be a problem if user call
pg_logical_slot_get_changes for a logical slot).

2)

WalSndWaitForStandbyConfirmation
...
+void
+WalSndWaitForStandbyConfirmation(XLogRecPtr wait_for_lsn)
+{
+ List *standby_slot_cpy;
+
+ if (!MyReplicationSlot->data.failover)
+ return;
+
+ standby_slot_cpy = list_copy(standby_slot_names_list);
+

The standby list could be un-initialized when calling from a non-walsender
backend (e.g. via pg_logical_slot_get_changes()). So, we need to call
SlotSyncInitConfig somewhere in this case.

Best Regards,
Hou zj