RE: Synchronizing slots from primary to standby

On Friday, October 20, 2023 11:27 AM shveta malik <shveta.malik@gmail.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