On Mon, Mar 30, 2026 at 3:52 PM shveta malik <[email protected]> wrote:
>
> On Mon, Mar 30, 2026 at 9:48 AM Nisha Moond <[email protected]> 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: v6-0001-Prevent-slotsync-worker-API-hang-during-standby-p.patch
Description: Binary data

Reply via email to