MIME-Version: 1.0
References: 
 <CAHGQGwFzNYroAxSoyJhqTU-pH=t4Ej6RyvhVmBZ91Exj_TPMMQ@mail.gmail.com>
 <CAA4eK1+CrQNqiPDKv1wYfdkbX0FARJoi1=0ioaAqkLzbq2vG1w@mail.gmail.com>
 <CAHGQGwHABvuCoyM24HUiFZ5oJq_CoFomjt_cqD-0cJLMjFXJjQ@mail.gmail.com>
 <CAA4eK1LzZGfRANPAnv6NpKCH2ENuZO6HswgY14A=xsOXmucPhw@mail.gmail.com>
 <CABdArM6nepct0uxizCnZqy-kAjjTOndvu7bWtaNcmknuxx82Hg@mail.gmail.com>
 <CAJpy0uBhJB4HqouLXegD=miSkZfZp86tTC2K0K6nf=bHCcsPLQ@mail.gmail.com>
 <CABdArM6Mfhk2+9TVR_D3cgfPWPfHDuZEg7MOc5KqULwt0OcQUg@mail.gmail.com>
 <CAA4eK1+d2vN80-Yvy_Hr=ATF3XL5db+_W-sXF=2Vxm+OFBO82w@mail.gmail.com>
 <CAA4eK1KxU4b53GUor41A55x+Bx-DdOaQ9g1DqURyRY2Cg-hhPQ@mail.gmail.com>
 <CABdArM6pmn5yFqiU33KTYBXYM=Vny2ULnJY_gqFbsMEdt+1dPA@mail.gmail.com>
 <CAHGQGwEsEjXbCthXb02=HScBW7C=BHmygv9SK1VGCcc-9bmTsw@mail.gmail.com>
 <CABdArM5rrhSmFvVL4C5LL0iea-R0HRtB=ZvD=ereoTDa1Tm=NA@mail.gmail.com>
In-Reply-To: 
 <CABdArM5rrhSmFvVL4C5LL0iea-R0HRtB=ZvD=ereoTDa1Tm=NA@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Mon, 30 Mar 2026 15:52:26 +0530
Message-ID: 
 <CAJpy0uD5brcAOpM8+UGfpzrm4pu-vsSfQUT2zgmcKsXEb8++4Q@mail.gmail.com>
Subject: Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during
 promotion?
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJpy0uD5brcAOpM8%2BUGfpzrm4pu-vsSfQUT2zgmcKsXEb8%2B%2B4Q%40mail.gmail.com>
Precedence: bulk

On Mon, Mar 30, 2026 at 9:48=E2=80=AFAM Nisha Moond <nisha.moond412@gmail.c=
om> 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().
 */

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.

Let me know if others have different opinions here.

thanks
Shveta