RE: promotion related handling in pg_sync_replication_slots()

On Monday, April 15, 2024 6:09 PM shveta malik <shveta.malik@gmail.com> wrote:
> 
> Please find v4 addressing the above comments.

Thanks for the patch.

Here are few comments:

1.

+            ereport(ERROR,
+                    errmsg("promotion in progress, can not synchronize replication slots"));
+        }

I think an errcode is needed.

The style of the error message seems a bit unnatural to me. I suggest:
"cannot synchronize replication slots when standby promotion is ongoing"


2.

+    if (worker_pid != InvalidPid)
+        Assert(SlotSyncCtx->pid == InvalidPid);

We could merge the checks into one Assert().
Assert(SlotSyncCtx->pid == InvalidPid || worker_pid == InvalidPid);


3.

-    pqsignal(SIGINT, SignalHandlerForShutdownRequest);

I realized that we should register this before setting SlotSyncCtx->pid,
otherwise if the standby is promoted after setting pid and before registering
signal handle function, the slotsync worker could miss to handle SIGINT sent by
startup process(ShutDownSlotSync). This is an existing issue for slotsync
worker, but maybe we could fix it together with the patch.

Best Regards,
Hou zj