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

From: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, shveta malik <shveta(dot)malik(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-04-08 09:45:10
Message-ID: CABdArM7B8Pnd+pZD201cZ0a1YfzoFytmbQw5XtEb84sD=rM92Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 8, 2026 at 12:24 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Wed, Apr 8, 2026 at 12:36 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> > The backpatch added PROCSIG_SLOTSYNC_MESSAGE in the middle of enum
> > ProcSignalReason, which could break the ABI. I’m planning to move it to
> > the end of the enum in v17 and v18.
> >
> > That seems to work for v18. However, in v17, NUM_PROCSIGNALS is defined
> > as the last enum value:
> >
> > NUM_PROCSIGNALS /* Must be last! */
> > } ProcSignalReason;
> >
> > So simply moving PROCSIG_SLOTSYNC_MESSAGE to the end would change the meaning
> > of NUM_PROCSIGNALS.
> >
> > One option might be to remove NUM_PROCSIGNALS from the enum, move
> > PROCSIG_SLOTSYNC_MESSAGE to the end, and define it separately, e.g.
> > #define NUM_PROCSIGNALS (PROCSIG_SLOTSYNC_MESSAGE + 1). Would that
> > be acceptable without breaking the ABI? Thoughts?
>
> The patches I'm planning to apply for v17 and v18 are attached.
>
> For v17, I'm still not entirely sure this change is safe from an ABI
> perspective. Even if it is, abi-compliance-check may still report
> a break since the patch removes NUM_PROCSIGNALS from the enum
> (defines it as separate macro). If so, we may need to update
> .abi-compliance-history to avoid false positives.
>

Regarding the pg17 change, NUM_PROCSIGNALS is not a process signal
reason but simply represents the array size, and its value will also
increase in pg18 (+1) after this backpatch.
AFAIU, the concern is that extensions might rely on the old compiled
values of PROCSIG_*, so we should avoid changing their order. However,
extensions should also not depend on NUM_PROCSIGNALS directly,
otherwise the pg18 backpatch would pose the same ABI concern. So, it
seems safe for pg17 as well.
I also checked core extensions and did not find NUM_PROCSIGNALS being used.

That said, I think both approaches - adding the new entry at the end
and defining NUM_PROCSIGNALS outside as done in the patch or adding it
just before NUM_PROCSIGNALS (like below) are semantically the same.
….
PROCSIG_RECOVERY_CONFLICT_LAST = PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
+ PROCSIG_SLOTSYNC_MESSAGE /* ask slot synchronization to stop */
+
NUM_PROCSIGNALS /* Must be last! */
} ProcSignalReason;

As NUM_PROCSIGNALS increments in both cases, I don’t see any
additional benefit in defining it outside. Thoughts? Happy to be
corrected if I’m missing something.

--
Thanks,
Nisha

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2026-04-08 09:58:23 Re: Eliminating SPI / SQL from some RI triggers - take 3
Previous Message Thomas Munro 2026-04-08 09:21:53 Refactoring DetermineSleepTime()