| 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 |
| 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 |