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: Fujii Masao <masao.fujii@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-04-08T09:45:10Z
Lists: 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



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Update .abi-compliance-history for change to enum ProcSignalReason

  2. Fix ABI break by moving PROCSIG_SLOTSYNC_MESSAGE in ProcSignalReason

  3. Fix slotsync worker blocking promotion when stuck in wait

  4. Add retry logic to pg_sync_replication_slots().

  5. Enhance slot synchronization API to respect promotion signal.