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