On Wed, Jul 16, 2025 at 3:47 PM Ajin Cherian <itsa...@gmail.com> wrote:
>
> > I am not able to apply the patch to the latest head or even to a week
> > back version. Can you please check and rebase?
> >
> > thanks
> > Shveta
>
> Rebased.
>
Thanks. Please find a few comments:
1)
/* Any slot with NULL in these fields should not have made it this far */
It is good to get rid of the case where we had checks for NULL
confirmed_lsn and catalog_xmin (i.e. when slot was in RS_EPHEMERAL
state), as that has already been checked by synchronize_slots() and
such a slot will not even reach wait_for_primary_slot_catchup(). But a
slot can still be invalidated on primary anytime, and thus during this
wait, we should check for primary's invalidation as we were doing in
v1.
2)
+ * If in SQL API synchronization, and we've been promoted, then no point
extra space before promoted.
3)
+ if (!AmLogicalSlotSyncWorkerProcess() && PromoteIsTriggered())
We don't need 'AmLogicalSlotSyncWorkerProcess' as that is already
checked at the beginning of this function.
4)
+ ereport(WARNING,
+ errmsg("aborting sync for slot \"%s\"",
+ remote_slot->name),
+ errdetail("Promotion occurred before this slot was fully"
+ " synchronized."));
+ pfree(cmd.data);
+
+ return false;
a) Please add an error-code.
b) Shall we change msg to
errmsg("aborting sync for slot \"%s\"",
remote_slot->name),
errhint("%s cannot be executed once promotion is
triggered.",
"pg_sync_replication_slots()")));
5)
Instead of using PromoteIsTriggered, shall we rely on
'SlotSyncCtx->stopSignaled' as we do when we start this API.
6)
In logicaldecoding.sgml, we can get rid of "Additionally, enabling
sync_replication_slots on the standby is required" to make it same as
what we had prior to the patch I pointed earlier.
Or better we can refine it to below. Thoughts?
The logical replication slots on the primary can be enabled for
synchronization to the hot standby by using the failover parameter of
pg_create_logical_replication_slot, or by using the failover option of
CREATE SUBSCRIPTION during slot creation. After that, synchronization
can be performed either manually by calling pg_sync_replication_slots
on the standby, or automatically by enabling sync_replication_slots on
the standby. When sync_replication_slots is enabled, the failover
slots are periodically synchronized by the slot sync worker. For the
synchronization to work, .....
thanks
Shveta
