Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?

On Fri, Mar 27, 2026 at 1:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Mar 27, 2026 at 10:27 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Fri, Mar 27, 2026 at 9:28 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > In ProcessSlotSyncInterrupts(), now we don't need the below logic right?
> > >
> > > if (SlotSyncCtx->stopSignaled)
> > > {
> > > if (AmLogicalSlotSyncWorkerProcess())
> > > {
> > > ...
> > > proc_exit(0);
> > > }
> > > else
> > > {
> > > /*
> > > * For the backend executing SQL function
> > > * pg_sync_replication_slots().
> > > */
> > > ereport(ERROR,
> > > errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> > > errmsg("replication slot synchronization will stop
> > > because promotion is triggered"));
> > > }
> > > }
> > >
> >
> > Right. Attached patch with the suggested changes.
> >
>
> After this change, why do we need to invoke
> ProcessSlotSyncInterrupts() twice in SyncReplicationSlots?
>

Also, not sure if it is a good idea to name current function as
ProcessSlotSyncInterrupts() because we remove most of its interrupt
handling. Shall we copy paste its code at two places as we do similar
handling at other places as well.

Another comment:
*
+
+ if (SlotSyncShutdown)
+ HandleSlotSyncShutdown();
...
...
+ if (CheckProcSignal(PROCSIG_SLOTSYNC_MESSAGE))
+ HandleSlotSyncShutdownInterrupt();

Would it better if we name these functions as HandleSlotSyncMessage()
and HandleSlotSyncMessageInterrupt() because for API, these simply
lead to an ERROR and that would match with the ProcSignalReason name
PROCSIG_SLOTSYNC_MESSAGE?

--
With Regards,
Amit Kapila.