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

From: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?
Date: 2026-03-31 06:01:41
Message-ID: CABdArM6E2aBj0FUjr2870sZUQqc6mpV1xSM=sqUFd2tKbfUQHA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 30, 2026 at 3:52 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Mar 30, 2026 at 9:48 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
>
> Thanks for the patch Nisha. Few trivial things:
>
> 1)
> + * Signal handler called (in signal context) when PROCSIG_SLOTSYNC_MESSAGE
> + * is received. Sets the SlotSyncShutdownPending flag so that
> ProcessInterrupts()
> + * will dispatch to ProcessSlotSyncMessage() at the next safe point.
> */
> +void
> +HandleSlotSyncMessageInterrupt(void)
>
> Can we please change the comment to below.
> Below suggestion is based on how we have written comments atop other
> such Handle*() functions
>
> /*
> * Handle receipt of an interrupt indicating a slotsync shutdown message
> *
> * This is called within SIGUSR1 handler. All we do here is set a flag
> * that will cause the next CHECK_FOR_INTERRUPTS() to invoke
> * ProcessSlotSyncMessage().
> */
>

Fixed.

> 2)
> I tried to consider whether 'stopSignaled' alone would be sufficient
> for our purposes, and whether we really need
> 'SlotSyncShutdownPending'. It seems that relying solely on
> stopSignaled could be problematic:
>
> a) stopSignaled resides in shared memory, so once it is set, it
> becomes visible to all other processes. If another process executes
> ProcessInterrupts(), it might incorrectly start handling slot sync
> shutdown. While this could be made to work, it would require extra
> checks to ensure that only the actual synchronizing process reacts.
> b) Unlike SlotSyncShutdownPending, we cannot reset stopSignaled, since
> it is also used to handle race conditions between the postmaster and
> promotion by the startup process.
> c) Accessing stopSignaled requires acquiring a mutex. It is unclear if
> that is a good idea in ProcessInterrupts(), since every time a process
> calls CHECK_FOR_INTERRUPTS(), it would need to acquire the mutex to
> decide whether to execute ProcessSlotSyncMessage().
>
> Given these considerations, I think it makes sense to retain both
> stopSignaled and SlotSyncShutdownPending, but we should add a comment
> above SlotSyncShutdownPending explaining why stopSignaled alone is not
> sufficient.
>

Done.

Please find the updated patch (v6) attached.

--
Thanks,
Nisha

Attachment Content-Type Size
v6-0001-Prevent-slotsync-worker-API-hang-during-standby-p.patch application/octet-stream 10.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2026-03-31 06:04:55 Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?
Previous Message cca5507 2026-03-31 04:36:32 Re: tuple radix sort