Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion? - Mailing list pgsql-hackers

From Nisha Moond
Subject Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?
Date
Msg-id CABdArM7B8Pnd+pZD201cZ0a1YfzoFytmbQw5XtEb84sD=rM92Q@mail.gmail.com
Whole thread
In response to Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?
List pgsql-hackers
On Wed, Apr 8, 2026 at 12:24 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Wed, Apr 8, 2026 at 12:36 PM Fujii Masao <masao.fujii@gmail.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



pgsql-hackers by date:

Previous
From: Dapeng Wang
Date:
Subject: Re: create table like including storage parameter
Next
From: Amit Langote
Date:
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3