RE: promotion related handling in pg_sync_replication_slots()

On Tuesday, April 16, 2024 2:52 PM Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:

Hi,

> On Tue, Apr 16, 2024 at 10:00:04AM +0530, shveta malik wrote:
> > Please find v5 addressing above comments.
>
> Thanks!
>
> @@ -1634,9 +1677,14 @@ SyncReplicationSlots(WalReceiverConn *wrconn) {
> PG_ENSURE_ERROR_CLEANUP(slotsync_failure_callback,
> PointerGetDatum(wrconn));
> {
> + check_flags_and_set_sync_info(InvalidPid);
> +
>
> Given the fact that if the sync worker is running it won't be possible to trigger a
> manual sync with pg_sync_replication_slots(), what about also checking the
> "sync_replication_slots" value at the start of SyncReplicationSlots() and emmit
> an error if sync_replication_slots is set to on? (The message could explicitly
> states that it's not possible to use the function if sync_replication_slots is set to
> on).

I personally feel adding the additional check for sync_replication_slots may
not improve the situation here. Because the GUC sync_replication_slots can
change at any point, the GUC could be false when performing this addition check
and is set to true immediately after the check, so It could not simplify the logic
anyway.

Best Regards,
Hou zj