On Tue, Apr 16, 2024 at 9:27 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Monday, April 15, 2024 6:09 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > Please find v4 addressing the above comments. > > Thanks for the patch. > > Here are few comments:
Thanks for reviewing the patch.
>
> 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"
Modified.
>
> 2.
>
> + if (worker_pid != InvalidPid)
> + Assert(SlotSyncCtx->pid == InvalidPid);
>
> We could merge the checks into one Assert().
> Assert(SlotSyncCtx->pid == InvalidPid || worker_pid == InvalidPid);
Modified.
>
> 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.
Yes, it seems like a problem. Fixed it. Also to be consistent, moved
other signal handlers' registration as well before we set pid.
Please find v5 addressing above comments.
thanks
Shveta
v5-0001-Handle-stopSignaled-during-sync-function-call.patch
Description: Binary data
