Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?
Nisha Moond <nisha.moond412@gmail.com>
From: Nisha Moond <nisha.moond412@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>,
Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-03-31T06:01:41Z
Lists: pgsql-hackers
Attachments
- v6-0001-Prevent-slotsync-worker-API-hang-during-standby-p.patch (application/octet-stream) patch v6-0001
On Mon, Mar 30, 2026 at 3:52 PM shveta malik <shveta.malik@gmail.com> wrote: > > On Mon, Mar 30, 2026 at 9:48 AM Nisha Moond <nisha.moond412@gmail.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
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Update .abi-compliance-history for change to enum ProcSignalReason
- 29d8bd908560 17.10 landed
-
Fix ABI break by moving PROCSIG_SLOTSYNC_MESSAGE in ProcSignalReason
- 586f4266fb49 17.10 landed
- acf49bfede2a 18.4 landed
-
Fix slotsync worker blocking promotion when stuck in wait
- 15910b1c363f 17.10 landed
- 58c1188a3eaa 18.4 landed
- db93032a7cbd 19 (unreleased) landed
-
Add retry logic to pg_sync_replication_slots().
- 0d2d4a0ec3ec 19 (unreleased) cited
-
Enhance slot synchronization API to respect promotion signal.
- 1362bc33e025 19 (unreleased) cited