Horton
Menu

Thread

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.

  1. Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-18T16:05:29Z

    Hi,

I noticed that during standby promotion the startup process sends SIGUSR1 to
the slotsync worker to make it exit. Is there a reason for using SIGUSR1?

If the slotsync worker is blocked waiting for input from the primary (e.g.,
due to a network outage between the primary and standby), SIGUSR1 won't
interrupt the wait. As a result, the worker can remain stuck and delay
promotion for a long time.

Would it make sense to send SIGTERM instead, so the worker can exit promptly
even while waiting? I've attached a WIP patch that does this. I haven't updated
the source comments yet, but I can do so if we agree on the approach.

SIGTERM alone is not sufficient, though. A new slotsync worker could start
immediately after the old one exits and block promotion again. To address this,
the patch makes a newly started worker exit immediately if promotion is
in progress.

Thoughts?

Regards,

-- 
Fujii Masao

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-18T16:23:51Z

    Fujii Masao <masao.fujii@gmail.com> writes:
> I noticed that during standby promotion the startup process sends SIGUSR1 to
> the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> Would it make sense to send SIGTERM instead, so the worker can exit promptly
> even while waiting?

One consideration here is that we expect all processes to receive
SIGTERM from init at the beginning of an operating system shutdown
sequence.  Background workers should exit at that point only if their
services will not be needed during database shutdown.  While it
sounds plausible that a slotsync worker should exit immediately,
I'm not quite sure if that's what we want.

			regards, tom lane

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-18T17:10:30Z

    On Thu, Mar 19, 2026 at 1:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fujii Masao <masao.fujii@gmail.com> writes:
> > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > Would it make sense to send SIGTERM instead, so the worker can exit promptly
> > even while waiting?
>
> One consideration here is that we expect all processes to receive
> SIGTERM from init at the beginning of an operating system shutdown
> sequence.  Background workers should exit at that point only if their
> services will not be needed during database shutdown.  While it
> sounds plausible that a slotsync worker should exit immediately,
> I'm not quite sure if that's what we want.

Currently, when the slotsync worker receives SIGUSR1 during promotion,
it exits at the next interrupt check (i.e., in ProcessSlotSyncInterrupts()).
There's no additional termination handling, so it seems the worker is expected
to exit promptly once the startup process requests it.

Given that, using SIGTERM to make the worker exit immediately seems OK to me...

With the patch, on SIGTERM, the worker basically still exits at the next
interrupt check. The difference is that if it's waiting for input from
the primary (i.e., DoingCommandRead = true), it calls ProcessInterrupts()
in the SIGTERM signal handler (die()) and exits immediately.

Regards.

-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-21T16:52:31Z

    On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> I noticed that during standby promotion the startup process sends SIGUSR1 to
> the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
>

IIRC, this same signal is used for both the backend executing
pg_sync_replication_slots() and slotsync worker. We want the worker to
exit and error_out backend. Using SIGTERM for backend could result in
its exit. Also, we want the last slotsync cycle to complete before
promotion so that chances of subscribers that do failover/switchover
to new primary has better chances of finding failover slots
sync-ready.

-- 
With Regards,
Amit Kapila.

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-23T05:50:52Z

    On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> >
>
> IIRC, this same signal is used for both the backend executing
> pg_sync_replication_slots() and slotsync worker. We want the worker to
> exit and error_out backend. Using SIGTERM for backend could result in
> its exit.

Why do we want the backend running pg_sync_replication_slots() to throw
an error here, rather than just exit? If emitting an error is really required,
another option would be to store the process type in SlotSyncCtx and send
different signals accordingly, for example, SIGTERM for the slotsync worker
and another signal for a backend. But it seems simpler and sufficient to have
the backend exit in this case as well.


> Also, we want the last slotsync cycle to complete before
> promotion so that chances of subscribers that do failover/switchover
> to new primary has better chances of finding failover slots
> sync-ready.

I'm not sure how much this behavior helps in failover/switchover scenarios.
But the main issue is that if a primary crash triggers standby promotion,
that last slotsync cycle can get stuck waiting for input from the primary,
which delays promotion. IOW, failover time can become unnecessarily long
due to the slotsync worker. I'd like to address that problem.

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-24T04:01:28Z

    On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > >
> >
> > IIRC, this same signal is used for both the backend executing
> > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > exit and error_out backend. Using SIGTERM for backend could result in
> > its exit.
>
> Why do we want the backend running pg_sync_replication_slots() to throw
> an error here, rather than just exit? If emitting an error is really required,
> another option would be to store the process type in SlotSyncCtx and send
> different signals accordingly, for example, SIGTERM for the slotsync worker
> and another signal for a backend. But it seems simpler and sufficient to have
> the backend exit in this case as well.
>
>
> > Also, we want the last slotsync cycle to complete before
> > promotion so that chances of subscribers that do failover/switchover
> > to new primary has better chances of finding failover slots
> > sync-ready.
>
> I'm not sure how much this behavior helps in failover/switchover scenarios.
> But the main issue is that if a primary crash triggers standby promotion,
> that last slotsync cycle can get stuck waiting for input from the primary,
> which delays promotion. IOW, failover time can become unnecessarily long
> due to the slotsync worker. I'd like to address that problem.
>

Hi Fujii-san,

I tried reproducing the wait scenario as you mentioned, but could not
reproduce it.
Steps I followed:
1) Place a debugger in the slotsync worker and hold it at
fetch_remote_slots() ... -> libpqsrv_get_result()
2) Kill the primary.
3) Triggered promotion of the standby and release debugger from slotsync worker.

The slot sync worker stops when the promotion is triggered and then
restarts, but fails to connect to the primary. The promotion happens
immediately.
```
LOG:  received promote request
LOG:  redo done at 0/0301AD40 system usage: CPU: user: 0.00 s, system:
0.02 s, elapsed: 4574.89 s
LOG:  last completed transaction was at log time 2026-03-23
17:13:15.782313+05:30
LOG:  replication slot synchronization worker will stop because
promotion is triggered
LOG:  slot sync worker started
ERROR:  synchronization worker "slotsync worker" could not connect to
the primary server: connection to server at "127.0.0.1", port 9933
failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?
```

I’ll debug this further to understand it better.
In the meantime, please let me know if I’m missing any step, or if you
followed a specific setup/script to reproduce this scenario.

--
Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-24T06:00:20Z

    On Tue, Mar 24, 2026 at 1:01 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> Hi Fujii-san,
>
> I tried reproducing the wait scenario as you mentioned, but could not
> reproduce it.
> Steps I followed:
> 1) Place a debugger in the slotsync worker and hold it at
> fetch_remote_slots() ... -> libpqsrv_get_result()
> 2) Kill the primary.
> 3) Triggered promotion of the standby and release debugger from slotsync worker.
>
> The slot sync worker stops when the promotion is triggered and then
> restarts, but fails to connect to the primary. The promotion happens
> immediately.
> ```
> LOG:  received promote request
> LOG:  redo done at 0/0301AD40 system usage: CPU: user: 0.00 s, system:
> 0.02 s, elapsed: 4574.89 s
> LOG:  last completed transaction was at log time 2026-03-23
> 17:13:15.782313+05:30
> LOG:  replication slot synchronization worker will stop because
> promotion is triggered
> LOG:  slot sync worker started
> ERROR:  synchronization worker "slotsync worker" could not connect to
> the primary server: connection to server at "127.0.0.1", port 9933
> failed: Connection refused
> Is the server running on that host and accepting TCP/IP connections?
> ```
>
> I’ll debug this further to understand it better.
> In the meantime, please let me know if I’m missing any step, or if you
> followed a specific setup/script to reproduce this scenario.

Thanks for testing!

If you killed the primary with a signal like SIGTERM, an RST packet might have
been sent to the slotsync worker at that moment. That allowed the worker to
detect the connection loss and exited the wait state, so promotion could
complete as expected.

To reproduce the issue, you'll need a scenario where the worker cannot detect
the connection loss. For example, you could block network traffic (e.g., with
iptables) between the primary and the slotsync worker. The key is to create
a situation where the worker remains stuck waiting for input for a long time.

Regards,

-- 
Fujii Masao

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-24T09:15:41Z

    On Tue, Mar 24, 2026 at 3:00 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Mar 24, 2026 at 1:01 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > Hi Fujii-san,
> >
> > I tried reproducing the wait scenario as you mentioned, but could not
> > reproduce it.
> > Steps I followed:
> > 1) Place a debugger in the slotsync worker and hold it at
> > fetch_remote_slots() ... -> libpqsrv_get_result()
> > 2) Kill the primary.
> > 3) Triggered promotion of the standby and release debugger from slotsync worker.
> >
> > The slot sync worker stops when the promotion is triggered and then
> > restarts, but fails to connect to the primary. The promotion happens
> > immediately.
> > ```
> > LOG:  received promote request
> > LOG:  redo done at 0/0301AD40 system usage: CPU: user: 0.00 s, system:
> > 0.02 s, elapsed: 4574.89 s
> > LOG:  last completed transaction was at log time 2026-03-23
> > 17:13:15.782313+05:30
> > LOG:  replication slot synchronization worker will stop because
> > promotion is triggered
> > LOG:  slot sync worker started
> > ERROR:  synchronization worker "slotsync worker" could not connect to
> > the primary server: connection to server at "127.0.0.1", port 9933
> > failed: Connection refused
> > Is the server running on that host and accepting TCP/IP connections?
> > ```
> >
> > I’ll debug this further to understand it better.
> > In the meantime, please let me know if I’m missing any step, or if you
> > followed a specific setup/script to reproduce this scenario.
>
> Thanks for testing!
>
> If you killed the primary with a signal like SIGTERM, an RST packet might have
> been sent to the slotsync worker at that moment. That allowed the worker to
> detect the connection loss and exited the wait state, so promotion could
> complete as expected.
>
> To reproduce the issue, you'll need a scenario where the worker cannot detect
> the connection loss. For example, you could block network traffic (e.g., with
> iptables) between the primary and the slotsync worker. The key is to create
> a situation where the worker remains stuck waiting for input for a long time.

Here's one way to reproduce the issue using iptables:

----------------------------------------------------
[Set up slot synchronization environment]

initdb -D data --encoding=UTF8 --locale=C
cat <<EOF >> data/postgresql.conf
wal_level = logical
synchronized_standby_slots = 'physical_slot'
EOF
pg_ctl -D data start
pg_receivewal --create-slot -S physical_slot
pg_recvlogical --create-slot -S logical_slot -P pgoutput
--enable-failover -d postgres
psql -c "CREATE PUBLICATION mypub"

pg_basebackup -D sby1 -c fast -R -S physical_slot -d "dbname=postgres"
-h 127.0.0.1
cat <<EOF >> sby1/postgresql.conf
port = 5433
sync_replication_slots = on
hot_standby_feedback = on
EOF
pg_ctl -D sby1 start

psql -c "SELECT pg_logical_emit_message(true, 'abc', 'xyz')"


[Block network traffic used by slot synchronization]
su -
iptables -A INPUT -p tcp --sport 5432 -j DROP
iptables -A OUTPUT -p tcp --dport 5432 -j DROP


[Promote the standby]
# wait a few seconds
pg_ctl -D sby1 promote
----------------------------------------------------

In my tests on master, promotion got stuck in this scenario.
With the patch, promotion completed promptly.

After testing, you can remove the network block with:

iptables -D INPUT -p tcp --sport 5432 -j DROP
iptables -D OUTPUT -p tcp --dport 5432 -j DROP

Regards,


-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-24T09:37:19Z

    On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > >
> >
> > IIRC, this same signal is used for both the backend executing
> > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > exit and error_out backend. Using SIGTERM for backend could result in
> > its exit.
>
> Why do we want the backend running pg_sync_replication_slots() to throw
> an error here, rather than just exit?
>

I think it was because the backends remain connected after promotion
and if we make them exit that will change the existing behavior.

-- 
With Regards,
Amit Kapila.

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-24T16:51:35Z

    On Tue, Mar 24, 2026 at 2:45 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Mar 24, 2026 at 3:00 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Tue, Mar 24, 2026 at 1:01 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > Hi Fujii-san,
> > >
> > > I tried reproducing the wait scenario as you mentioned, but could not
> > > reproduce it.
> > > Steps I followed:
> > > 1) Place a debugger in the slotsync worker and hold it at
> > > fetch_remote_slots() ... -> libpqsrv_get_result()
> > > 2) Kill the primary.
> > > 3) Triggered promotion of the standby and release debugger from slotsync worker.
> > >
> > > The slot sync worker stops when the promotion is triggered and then
> > > restarts, but fails to connect to the primary. The promotion happens
> > > immediately.
> > > ```
> > > LOG:  received promote request
> > > LOG:  redo done at 0/0301AD40 system usage: CPU: user: 0.00 s, system:
> > > 0.02 s, elapsed: 4574.89 s
> > > LOG:  last completed transaction was at log time 2026-03-23
> > > 17:13:15.782313+05:30
> > > LOG:  replication slot synchronization worker will stop because
> > > promotion is triggered
> > > LOG:  slot sync worker started
> > > ERROR:  synchronization worker "slotsync worker" could not connect to
> > > the primary server: connection to server at "127.0.0.1", port 9933
> > > failed: Connection refused
> > > Is the server running on that host and accepting TCP/IP connections?
> > > ```
> > >
> > > I’ll debug this further to understand it better.
> > > In the meantime, please let me know if I’m missing any step, or if you
> > > followed a specific setup/script to reproduce this scenario.
> >
> > Thanks for testing!
> >
> > If you killed the primary with a signal like SIGTERM, an RST packet might have
> > been sent to the slotsync worker at that moment. That allowed the worker to
> > detect the connection loss and exited the wait state, so promotion could
> > complete as expected.
> >
> > To reproduce the issue, you'll need a scenario where the worker cannot detect
> > the connection loss. For example, you could block network traffic (e.g., with
> > iptables) between the primary and the slotsync worker. The key is to create
> > a situation where the worker remains stuck waiting for input for a long time.
>
> Here's one way to reproduce the issue using iptables:
>

Thank you, Fujii-san, for sharing the steps. I am now able to
reproduce the behavior where promotion gets stuck because the slot
sync worker remains in a wait loop.

As an experiment, I tried setting tcp_user_timeout to 7000 / 15000
(using slightly higher values for debugging). With this setting, the
TCP stack terminates the connection if data sent to the primary
remains unacknowledged beyond the configured timeout (e.g., due to a
network drop). In such cases the slot sync worker exits instead of
waiting indefinitely. With an appropriately tuned timeout, this could
help avoid the promotion issue by ensuring the worker does not remain
stuck when the connection to the primary is lost.

Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-25T00:51:17Z

    On Wed, Mar 25, 2026 at 1:51 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
> Thank you, Fujii-san, for sharing the steps. I am now able to
> reproduce the behavior where promotion gets stuck because the slot
> sync worker remains in a wait loop.

Thanks for the test!


> As an experiment, I tried setting tcp_user_timeout to 7000 / 15000
> (using slightly higher values for debugging). With this setting, the
> TCP stack terminates the connection if data sent to the primary
> remains unacknowledged beyond the configured timeout (e.g., due to a
> network drop). In such cases the slot sync worker exits instead of
> waiting indefinitely. With an appropriately tuned timeout, this could
> help avoid the promotion issue by ensuring the worker does not remain
> stuck when the connection to the primary is lost.

Yes, TCP timeout settings like tcp_user_timeout, keepalives,
and net.ipv4.tcp_retries2 can help in this situation. However,
they involve a trade-off: using very small timeouts can reduce
failover time but increases the risk of false network failure detection,
while larger timeouts (e.g., 10s) avoid false positives but can
delay failover by that amount.

Because of this, I think it's better to address the issue without
relying on such TCP timeout parameters.

Also, tcp_user_timeout is not available on platforms that don't
support TCP_USER_TIMEOUT (e.g., Windows).

Regards,

-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-26T10:10:10Z

    On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > >
> >
> > IIRC, this same signal is used for both the backend executing
> > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > exit and error_out backend. Using SIGTERM for backend could result in
> > its exit.
>
> Why do we want the backend running pg_sync_replication_slots() to throw
> an error here, rather than just exit? If emitting an error is really required,
> another option would be to store the process type in SlotSyncCtx and send
> different signals accordingly, for example, SIGTERM for the slotsync worker
> and another signal for a backend. But it seems simpler and sufficient to have
> the backend exit in this case as well.
>

As we want to retain the existing behavior for API, so instead of
using two signals, we can achieve what you intend to achieve by one
signal (SIGUSR1) only. We can use SendProcSignal mechanism as is used
ParallelWorkerShutdown. On promotion, we send a SIGUSR1 signal to
slotsync worker/backend via SendProcSignal. Then in
procsignal_sigusr1_handler(), it will call HandleSlotSyncInterrupt.
HandleSlotSyncInterrupt() will set the InterruptPending and
SlotSyncPending flag. Then ProcessInterrupt() will call a slotsync
specific function based on the flag and do what we currently do in
ProcessSlotSyncInterrupts. I think this should address the issue you
are worried about.

-- 
With Regards,
Amit Kapila.

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-26T10:37:44Z

    On Thu, Mar 26, 2026 at 3:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > >
> > > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > > >
> > >
> > > IIRC, this same signal is used for both the backend executing
> > > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > > exit and error_out backend. Using SIGTERM for backend could result in
> > > its exit.
> >
> > Why do we want the backend running pg_sync_replication_slots() to throw
> > an error here, rather than just exit? If emitting an error is really required,
> > another option would be to store the process type in SlotSyncCtx and send
> > different signals accordingly, for example, SIGTERM for the slotsync worker
> > and another signal for a backend. But it seems simpler and sufficient to have
> > the backend exit in this case as well.
> >
>
> As we want to retain the existing behavior for API, so instead of
> using two signals, we can achieve what you intend to achieve by one
> signal (SIGUSR1) only. We can use SendProcSignal mechanism as is used
> ParallelWorkerShutdown. On promotion, we send a SIGUSR1 signal to
> slotsync worker/backend via SendProcSignal. Then in
> procsignal_sigusr1_handler(), it will call HandleSlotSyncInterrupt.
> HandleSlotSyncInterrupt() will set the InterruptPending and
> SlotSyncPending flag. Then ProcessInterrupt() will call a slotsync
> specific function based on the flag and do what we currently do in
> ProcessSlotSyncInterrupts. I think this should address the issue you
> are worried about.
>

+1
Retaining the current behavior for the API backend keeps it consistent
with other backends that continue after promotion.

In the reproduced case, the worker (or API backend) is waiting in:
libpqsrv_get_result -> WaitLatchOrSocket -> WaitEventSetWait.
When SIGUSR1 is received, it only sets the latch but does not mark any
interrupt as pending. As a result, CHECK_FOR_INTERRUPTS() is
effectively a no-op, and the process goes back to waiting. So, control
never returns to the slotsync code path, and we cannot rely on
stopSignaled to handle exit/error separately.
Only SIGTERM works here because its handler sets
INTERRUPTS_PENDING_CONDITION, allowing ProcessInterrupts() to run and
break the loop. The other signals like SIGUSR1 or SIGINT do not do
this, so simply using another signal might not solve the API error
handling case.

I’ve implemented the above approach suggested by Amit in the attached
patch and verified it for both worker and API scenarios. With this,
the API can now error-out without exiting the backend.

--
Thanks,
Nisha

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

    shveta malik <shveta.malik@gmail.com> — 2026-03-27T03:58:41Z

    On Thu, Mar 26, 2026 at 4:08 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, Mar 26, 2026 at 3:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > >
> > > > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > > > >
> > > >
> > > > IIRC, this same signal is used for both the backend executing
> > > > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > > > exit and error_out backend. Using SIGTERM for backend could result in
> > > > its exit.
> > >
> > > Why do we want the backend running pg_sync_replication_slots() to throw
> > > an error here, rather than just exit? If emitting an error is really required,
> > > another option would be to store the process type in SlotSyncCtx and send
> > > different signals accordingly, for example, SIGTERM for the slotsync worker
> > > and another signal for a backend. But it seems simpler and sufficient to have
> > > the backend exit in this case as well.
> > >
> >
> > As we want to retain the existing behavior for API, so instead of
> > using two signals, we can achieve what you intend to achieve by one
> > signal (SIGUSR1) only. We can use SendProcSignal mechanism as is used
> > ParallelWorkerShutdown. On promotion, we send a SIGUSR1 signal to
> > slotsync worker/backend via SendProcSignal. Then in
> > procsignal_sigusr1_handler(), it will call HandleSlotSyncInterrupt.
> > HandleSlotSyncInterrupt() will set the InterruptPending and
> > SlotSyncPending flag. Then ProcessInterrupt() will call a slotsync
> > specific function based on the flag and do what we currently do in
> > ProcessSlotSyncInterrupts. I think this should address the issue you
> > are worried about.
> >
>
> +1
> Retaining the current behavior for the API backend keeps it consistent
> with other backends that continue after promotion.
>
> In the reproduced case, the worker (or API backend) is waiting in:
> libpqsrv_get_result -> WaitLatchOrSocket -> WaitEventSetWait.
> When SIGUSR1 is received, it only sets the latch but does not mark any
> interrupt as pending. As a result, CHECK_FOR_INTERRUPTS() is
> effectively a no-op, and the process goes back to waiting. So, control
> never returns to the slotsync code path, and we cannot rely on
> stopSignaled to handle exit/error separately.
> Only SIGTERM works here because its handler sets
> INTERRUPTS_PENDING_CONDITION, allowing ProcessInterrupts() to run and
> break the loop. The other signals like SIGUSR1 or SIGINT do not do
> this, so simply using another signal might not solve the API error
> handling case.
>
> I’ve implemented the above approach suggested by Amit in the attached
> patch and verified it for both worker and API scenarios. With this,
> the API can now error-out without exiting the backend.
>

+1 on the idea. Few comments:

1)
It was not clear initially as to why SetLatch is not done in
HandleSlotSyncShutdownInterrupt(), digging it further revealed that
procsignal_sigusr1_handler() will do SetLatch outside. Perhaps you can
add below comment at the end of HandleSlotSyncShutdownInterrupt()
similar to how other functions (HandleProcSignalBarrierInterrupt,
HandleRecoveryConflictInterrupt etc) do.

/* latch will be set by procsignal_sigusr1_handler */

2)
In ProcessSlotSyncInterrupts(), now we don't need the below logic right?

if (SlotSyncCtx->stopSignaled)
    {
        if (AmLogicalSlotSyncWorkerProcess())
        {
            ...
            proc_exit(0);
        }
        else
        {
            /*
             * For the backend executing SQL function
             * pg_sync_replication_slots().
             */
            ereport(ERROR,
                    errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                    errmsg("replication slot synchronization will stop
because promotion is triggered"));
        }
    }

thanks
Shveta

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-27T04:57:43Z

    On Fri, Mar 27, 2026 at 9:28 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Mar 26, 2026 at 4:08 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Thu, Mar 26, 2026 at 3:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Mon, Mar 23, 2026 at 11:21 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > >
> > > > On Sun, Mar 22, 2026 at 1:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > On Wed, Mar 18, 2026 at 9:35 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > > >
> > > > > > I noticed that during standby promotion the startup process sends SIGUSR1 to
> > > > > > the slotsync worker to make it exit. Is there a reason for using SIGUSR1?
> > > > > >
> > > > >
> > > > > IIRC, this same signal is used for both the backend executing
> > > > > pg_sync_replication_slots() and slotsync worker. We want the worker to
> > > > > exit and error_out backend. Using SIGTERM for backend could result in
> > > > > its exit.
> > > >
> > > > Why do we want the backend running pg_sync_replication_slots() to throw
> > > > an error here, rather than just exit? If emitting an error is really required,
> > > > another option would be to store the process type in SlotSyncCtx and send
> > > > different signals accordingly, for example, SIGTERM for the slotsync worker
> > > > and another signal for a backend. But it seems simpler and sufficient to have
> > > > the backend exit in this case as well.
> > > >
> > >
> > > As we want to retain the existing behavior for API, so instead of
> > > using two signals, we can achieve what you intend to achieve by one
> > > signal (SIGUSR1) only. We can use SendProcSignal mechanism as is used
> > > ParallelWorkerShutdown. On promotion, we send a SIGUSR1 signal to
> > > slotsync worker/backend via SendProcSignal. Then in
> > > procsignal_sigusr1_handler(), it will call HandleSlotSyncInterrupt.
> > > HandleSlotSyncInterrupt() will set the InterruptPending and
> > > SlotSyncPending flag. Then ProcessInterrupt() will call a slotsync
> > > specific function based on the flag and do what we currently do in
> > > ProcessSlotSyncInterrupts. I think this should address the issue you
> > > are worried about.
> > >
> >
> > +1
> > Retaining the current behavior for the API backend keeps it consistent
> > with other backends that continue after promotion.
> >
> > In the reproduced case, the worker (or API backend) is waiting in:
> > libpqsrv_get_result -> WaitLatchOrSocket -> WaitEventSetWait.
> > When SIGUSR1 is received, it only sets the latch but does not mark any
> > interrupt as pending. As a result, CHECK_FOR_INTERRUPTS() is
> > effectively a no-op, and the process goes back to waiting. So, control
> > never returns to the slotsync code path, and we cannot rely on
> > stopSignaled to handle exit/error separately.
> > Only SIGTERM works here because its handler sets
> > INTERRUPTS_PENDING_CONDITION, allowing ProcessInterrupts() to run and
> > break the loop. The other signals like SIGUSR1 or SIGINT do not do
> > this, so simply using another signal might not solve the API error
> > handling case.
> >
> > I’ve implemented the above approach suggested by Amit in the attached
> > patch and verified it for both worker and API scenarios. With this,
> > the API can now error-out without exiting the backend.
> >
>
> +1 on the idea. Few comments:
>

Thanks for the review.

> 1)
> It was not clear initially as to why SetLatch is not done in
> HandleSlotSyncShutdownInterrupt(), digging it further revealed that
> procsignal_sigusr1_handler() will do SetLatch outside. Perhaps you can
> add below comment at the end of HandleSlotSyncShutdownInterrupt()
> similar to how other functions (HandleProcSignalBarrierInterrupt,
> HandleRecoveryConflictInterrupt etc) do.
>
> /* latch will be set by procsignal_sigusr1_handler */
>

Fixed.

> 2)
> In ProcessSlotSyncInterrupts(), now we don't need the below logic right?
>
> if (SlotSyncCtx->stopSignaled)
>     {
>         if (AmLogicalSlotSyncWorkerProcess())
>         {
>             ...
>             proc_exit(0);
>         }
>         else
>         {
>             /*
>              * For the backend executing SQL function
>              * pg_sync_replication_slots().
>              */
>             ereport(ERROR,
>                     errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>                     errmsg("replication slot synchronization will stop
> because promotion is triggered"));
>         }
>     }
>

Right. Attached patch with the suggested changes.

--
Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-27T06:06:17Z

    On Fri, Mar 27, 2026 at 1:57 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Right. Attached patch with the suggested changes.
>

Thanks for making the patch!

From a quick look, this approach seems fine to me. I'll review it in
more detail later.

Thanks for working on this issue!

Regards,

-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-27T07:49:58Z

    On Fri, Mar 27, 2026 at 10:27 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Fri, Mar 27, 2026 at 9:28 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > In ProcessSlotSyncInterrupts(), now we don't need the below logic right?
> >
> > if (SlotSyncCtx->stopSignaled)
> >     {
> >         if (AmLogicalSlotSyncWorkerProcess())
> >         {
> >             ...
> >             proc_exit(0);
> >         }
> >         else
> >         {
> >             /*
> >              * For the backend executing SQL function
> >              * pg_sync_replication_slots().
> >              */
> >             ereport(ERROR,
> >                     errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> >                     errmsg("replication slot synchronization will stop
> > because promotion is triggered"));
> >         }
> >     }
> >
>
> Right. Attached patch with the suggested changes.
>

After this change, why do we need to invoke
ProcessSlotSyncInterrupts() twice in SyncReplicationSlots?

-- 
With Regards,
Amit Kapila.

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-27T09:49:00Z

    On Fri, Mar 27, 2026 at 1:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Mar 27, 2026 at 10:27 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Fri, Mar 27, 2026 at 9:28 AM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > In ProcessSlotSyncInterrupts(), now we don't need the below logic right?
> > >
> > > if (SlotSyncCtx->stopSignaled)
> > >     {
> > >         if (AmLogicalSlotSyncWorkerProcess())
> > >         {
> > >             ...
> > >             proc_exit(0);
> > >         }
> > >         else
> > >         {
> > >             /*
> > >              * For the backend executing SQL function
> > >              * pg_sync_replication_slots().
> > >              */
> > >             ereport(ERROR,
> > >                     errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> > >                     errmsg("replication slot synchronization will stop
> > > because promotion is triggered"));
> > >         }
> > >     }
> > >
> >
> > Right. Attached patch with the suggested changes.
> >
>
> After this change, why do we need to invoke
> ProcessSlotSyncInterrupts() twice in SyncReplicationSlots?
>

Also, not sure if it is a good idea to name current function as
ProcessSlotSyncInterrupts() because we remove most of its interrupt
handling. Shall we copy paste its code at two places as we do similar
handling at other places as well.

Another comment:
*
+
+ if (SlotSyncShutdown)
+ HandleSlotSyncShutdown();
...
...
+ if (CheckProcSignal(PROCSIG_SLOTSYNC_MESSAGE))
+ HandleSlotSyncShutdownInterrupt();

Would it better if we name these functions as HandleSlotSyncMessage()
and HandleSlotSyncMessageInterrupt() because for API, these simply
lead to an ERROR and that would match with the ProcSignalReason name
PROCSIG_SLOTSYNC_MESSAGE?

-- 
With Regards,
Amit Kapila.

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-27T12:37:56Z

    On Fri, Mar 27, 2026 at 3:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Mar 27, 2026 at 1:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > After this change, why do we need to invoke
> > ProcessSlotSyncInterrupts() twice in SyncReplicationSlots?
> >

Fixed.

>
> Also, not sure if it is a good idea to name current function as
> ProcessSlotSyncInterrupts() because we remove most of its interrupt
> handling. Shall we copy paste its code at two places as we do similar
> handling at other places as well.
>

Done.

> Another comment:
> *
> +
> + if (SlotSyncShutdown)
> + HandleSlotSyncShutdown();
> ...
> ...
> + if (CheckProcSignal(PROCSIG_SLOTSYNC_MESSAGE))
> + HandleSlotSyncShutdownInterrupt();
>
> Would it better if we name these functions as HandleSlotSyncMessage()
> and HandleSlotSyncMessageInterrupt() because for API, these simply
> lead to an ERROR and that would match with the ProcSignalReason name
> PROCSIG_SLOTSYNC_MESSAGE?
>
Done.

Attached the updated patch.

--
Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-27T17:19:26Z

    On Fri, Mar 27, 2026 at 9:38 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> Attached the updated patch.

Thanks for updating the patch! It looks good overall.

Regarding the comments in SlotSyncCtxStruct, since the role of
stopSignaled field has changed, those comments should be updated
accordingly? For example,

-------------------------
- * the SQL function pg_sync_replication_slots(). When the startup process sets
- * 'stopSignaled' during promotion, it uses this 'pid' to wake up the currently
- * synchronizing process so that the process can immediately stop its
- * synchronizing work on seeing 'stopSignaled' set.
- * Setting 'stopSignaled' is also used to handle the race condition when the
+ * the SQL function pg_sync_replication_slots(). On promotion,
+ * the startup process sets 'stopSignaled' and uses this 'pid' to wake up
+ * the currently synchronizing process so that the process can
+ * immediately stop its synchronizing work.
+ * Setting 'stopSignaled' is used to handle the race condition when the
-------------------------


+/*
+ * Interrupt flag set when PROCSIG_SLOTSYNC_MESSAGE is received, asking the
+ * slotsync worker or pg_sync_replication_slots() to stop because
+ * standby promotion has been triggered.
+ */
+volatile sig_atomic_t SlotSyncShutdown = false;

For the interrupt flag set in procsignal_sigusr1_handler(), other flags
use a *Pending suffix (e.g., ProcSignalBarrierPending,
ParallelApplyMessagePending), so SlotSyncShutdownPending would
be more consistent.


+void
+HandleSlotSyncMessage(void)

Functions called from ProcessInterrupts() typically use the Process* prefix
(e.g., ProcessProcSignalBarrier(), ProcessParallelApplyMessages()),
so ProcessSlotSyncMessage would be more consistent than HandleSlotSyncMessage.


+ ereport(LOG,
+ errmsg("replication slot synchronization worker will stop because
promotion is triggered"));
+
+ proc_exit(0);
+ }
+ else
+ {
+ /*
+ * For the backend executing SQL function
+ * pg_sync_replication_slots().
+ */
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("replication slot synchronization will stop because promotion
is triggered"));

The log messages say "will stop", but since sync hasn't started yet,
"will not start" seems clearer here. For example, "replication slot
synchronization worker will not start because promotion was triggered"
and "replication slot synchronization will not start because promotion was
triggered". Thought?

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-30T04:18:19Z

    On Fri, Mar 27, 2026 at 10:49 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Mar 27, 2026 at 9:38 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > Attached the updated patch.
>
> Thanks for updating the patch! It looks good overall.
>

Thank you Fujii-san for the review.

> Regarding the comments in SlotSyncCtxStruct, since the role of
> stopSignaled field has changed, those comments should be updated
> accordingly? For example,
>
> -------------------------
> - * the SQL function pg_sync_replication_slots(). When the startup process sets
> - * 'stopSignaled' during promotion, it uses this 'pid' to wake up the currently
> - * synchronizing process so that the process can immediately stop its
> - * synchronizing work on seeing 'stopSignaled' set.
> - * Setting 'stopSignaled' is also used to handle the race condition when the
> + * the SQL function pg_sync_replication_slots(). On promotion,
> + * the startup process sets 'stopSignaled' and uses this 'pid' to wake up
> + * the currently synchronizing process so that the process can
> + * immediately stop its synchronizing work.
> + * Setting 'stopSignaled' is used to handle the race condition when the
> -------------------------
>

Updated as suggested.

>
> +/*
> + * Interrupt flag set when PROCSIG_SLOTSYNC_MESSAGE is received, asking the
> + * slotsync worker or pg_sync_replication_slots() to stop because
> + * standby promotion has been triggered.
> + */
> +volatile sig_atomic_t SlotSyncShutdown = false;
>
> For the interrupt flag set in procsignal_sigusr1_handler(), other flags
> use a *Pending suffix (e.g., ProcSignalBarrierPending,
> ParallelApplyMessagePending), so SlotSyncShutdownPending would
> be more consistent.
>
>
> +void
> +HandleSlotSyncMessage(void)
>
> Functions called from ProcessInterrupts() typically use the Process* prefix
> (e.g., ProcessProcSignalBarrier(), ProcessParallelApplyMessages()),
> so ProcessSlotSyncMessage would be more consistent than HandleSlotSyncMessage.
>

Agree, fixed.

>
> + ereport(LOG,
> + errmsg("replication slot synchronization worker will stop because
> promotion is triggered"));
> +
> + proc_exit(0);
> + }
> + else
> + {
> + /*
> + * For the backend executing SQL function
> + * pg_sync_replication_slots().
> + */
> + ereport(ERROR,
> + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("replication slot synchronization will stop because promotion
> is triggered"));
>
> The log messages say "will stop", but since sync hasn't started yet,
> "will not start" seems clearer here. For example, "replication slot
> synchronization worker will not start because promotion was triggered"
> and "replication slot synchronization will not start because promotion was
> triggered". Thought?
>

We were using the same log message in two places:
check_and_set_sync_info() and HandleSlotSyncMessage().
I think “will not start” fits better in the first case, while “will
stop” makes sense to keep in the second.

--
Thanks,
Nisha

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

    shveta malik <shveta.malik@gmail.com> — 2026-03-30T10:22:26Z

    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().
 */

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

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-30T11:09:20Z

    On Mon, Mar 30, 2026 at 1:18 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> We were using the same log message in two places:
> check_and_set_sync_info() and HandleSlotSyncMessage().
> I think “will not start” fits better in the first case, while “will
> stop” makes sense to keep in the second.

Thanks for updating the patch!

With the patch, in my testing, standby promotion always produces
the following logs:

    LOG:  replication slot synchronization worker will stop because
promotion is triggered
    LOG:  replication slot synchronization worker will not start
because promotion was triggered

It looks like the postmaster immediately restarts the slotsync worker after
promotion terminates it, and that new worker then exits on seeing
SlotSyncCtx->stopSignaled.

IMO, always emitting both messages is a bit confusing. It would be nice to
suppress the second one if possible.

One idea would be to prevent the restart altogether. For example,
ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
a special value (like -1), and SlotSyncWorkerCanRestart() could return
false (i.e., prevent postmater from starting up slotsync worker) when
it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
check SlotSyncCtx->stopSignaled.

That said, as far as I remember correctly, postmaster is generally not
supposed to touch shared memory (per the comments in postmaster.c),
so I'm not sure this approach is acceptable. On the other hand,
postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
so perhaps there's some precedent here.

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-31T06:01:41Z

    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

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-03-31T06:04:55Z

    On Mon, Mar 30, 2026 at 4:39 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Mon, Mar 30, 2026 at 1:18 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > We were using the same log message in two places:
> > check_and_set_sync_info() and HandleSlotSyncMessage().
> > I think “will not start” fits better in the first case, while “will
> > stop” makes sense to keep in the second.
>
> Thanks for updating the patch!
>
> With the patch, in my testing, standby promotion always produces
> the following logs:
>
>     LOG:  replication slot synchronization worker will stop because
> promotion is triggered
>     LOG:  replication slot synchronization worker will not start
> because promotion was triggered
>
> It looks like the postmaster immediately restarts the slotsync worker after
> promotion terminates it, and that new worker then exits on seeing
> SlotSyncCtx->stopSignaled.
>
> IMO, always emitting both messages is a bit confusing. It would be nice to
> suppress the second one if possible.
>
> One idea would be to prevent the restart altogether. For example,
> ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> a special value (like -1), and SlotSyncWorkerCanRestart() could return
> false (i.e., prevent postmater from starting up slotsync worker) when
> it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> check SlotSyncCtx->stopSignaled.
>
> That said, as far as I remember correctly, postmaster is generally not
> supposed to touch shared memory (per the comments in postmaster.c),
> so I'm not sure this approach is acceptable. On the other hand,
> postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
> so perhaps there's some precedent here.
>
IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
may not be ideal, as it requires a spinlock to avoid races with the
startup process and it is disallowed to take lock in postmaster main
loop. Whereas, SlotSyncCtx->last_start_time doesn’t need a lock since
the postmaster accesses it only when the worker is not alive.

Another option could be to log in check_and_set_sync_info() at DEBUG1
instead of LOG level. This message appears only after stopSignaled is
set, when promotion is already in progress and the first worker has
logged “will stop…”. The second worker doesn’t do any real work. Since
there’s nothing actionable for users, using DEBUG1 would keep it
useful for debugging (e.g., noticing immediate restarts) while
avoiding extra log noise. Thoughts?

--
Thanks,
Nisha

  26. RE: Use SIGTERM instead of SIGUSR1 for slotsync worker to exit during promotion?

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-31T10:26:33Z

    On Tuesday, March 31, 2026 2:02 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> 
> 
> Please find the updated patch (v6) attached.

Thanks for updating the patch. One minor comment:

I think we could avoid interrupting and reporting an ERROR when
IsSyncingReplicationSlots() returns false to avoid reporting ERROR unnecessarily
when the slotsync has already finished.

Best Regards,
Hou zj

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

    shveta malik <shveta.malik@gmail.com> — 2026-03-31T10:42:00Z

    On Tue, Mar 31, 2026 at 11:35 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Mon, Mar 30, 2026 at 4:39 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Mon, Mar 30, 2026 at 1:18 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > We were using the same log message in two places:
> > > check_and_set_sync_info() and HandleSlotSyncMessage().
> > > I think “will not start” fits better in the first case, while “will
> > > stop” makes sense to keep in the second.
> >
> > Thanks for updating the patch!
> >
> > With the patch, in my testing, standby promotion always produces
> > the following logs:
> >
> >     LOG:  replication slot synchronization worker will stop because
> > promotion is triggered
> >     LOG:  replication slot synchronization worker will not start
> > because promotion was triggered
> >
> > It looks like the postmaster immediately restarts the slotsync worker after
> > promotion terminates it, and that new worker then exits on seeing
> > SlotSyncCtx->stopSignaled.
> >
> > IMO, always emitting both messages is a bit confusing. It would be nice to
> > suppress the second one if possible.
> >
> > One idea would be to prevent the restart altogether. For example,
> > ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> > a special value (like -1), and SlotSyncWorkerCanRestart() could return
> > false (i.e., prevent postmater from starting up slotsync worker) when
> > it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> > check SlotSyncCtx->stopSignaled.
> >
> > That said, as far as I remember correctly, postmaster is generally not
> > supposed to touch shared memory (per the comments in postmaster.c),
> > so I'm not sure this approach is acceptable. On the other hand,
> > postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
> > so perhaps there's some precedent here.
> >
> IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
> may not be ideal, as it requires a spinlock to avoid races with the
> startup process and it is disallowed to take lock in postmaster main
> loop. Whereas, SlotSyncCtx->last_start_time doesn’t need a lock since
> the postmaster accesses it only when the worker is not alive.
>

I agree.

> Another option could be to log in check_and_set_sync_info() at DEBUG1
> instead of LOG level. This message appears only after stopSignaled is
> set, when promotion is already in progress and the first worker has
> logged “will stop…”. The second worker doesn’t do any real work. Since
> there’s nothing actionable for users, using DEBUG1 would keep it
> useful for debugging (e.g., noticing immediate restarts) while
> avoiding extra log noise. Thoughts?
>

+1.

Do you think we can slightly tweak the comment in atop file to:

On promotion the startup process sets 'stopSignaled' and uses this
'pid' to signal synchronizing process with PROCSIG_SLOTSYNC_MESSAGE
and also to wake it up so that the process can immediately stop its
synchronizing work. Setting 'stopSignaled' on the other hand is  used
to handle the race condition....

Also shall we quick exit ProcessSlotSyncMessage() if syncing is
already finished by API?

thanks
Shveta

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-03-31T15:33:27Z

    On Tue, Mar 31, 2026 at 7:42 PM shveta malik <shveta.malik@gmail.com> wrote:
> > > One idea would be to prevent the restart altogether. For example,
> > > ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> > > a special value (like -1), and SlotSyncWorkerCanRestart() could return
> > > false (i.e., prevent postmater from starting up slotsync worker) when
> > > it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> > > check SlotSyncCtx->stopSignaled.
> > >
> > > That said, as far as I remember correctly, postmaster is generally not
> > > supposed to touch shared memory (per the comments in postmaster.c),
> > > so I'm not sure this approach is acceptable. On the other hand,
> > > postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
> > > so perhaps there's some precedent here.
> > >
> > IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
> > may not be ideal, as it requires a spinlock to avoid races with the
> > startup process and it is disallowed to take lock in postmaster main
> > loop. Whereas, SlotSyncCtx->last_start_time doesn’t need a lock since
> > the postmaster accesses it only when the worker is not alive.
> >
>
> I agree.

Could you clarify what issue might arise from checking
SlotSyncCtx->stopSignaled without holding a spinlock in
SlotSyncWorkerCanRestart()? Is it actually problematic?

That said, since the postmaster should generally avoid
touching shared memory, it doesn't seem like a good idea
for it to check SlotSyncCtx->stopSignaled. So I'm fine with
instead lowering the log level for the "worker will not start"
message to DEBUG1.

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-01T05:03:43Z

    On Tue, Mar 31, 2026 at 9:03 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Mar 31, 2026 at 7:42 PM shveta malik <shveta.malik@gmail.com> wrote:
> > > > One idea would be to prevent the restart altogether. For example,
> > > > ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> > > > a special value (like -1), and SlotSyncWorkerCanRestart() could return
> > > > false (i.e., prevent postmater from starting up slotsync worker) when
> > > > it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> > > > check SlotSyncCtx->stopSignaled.
> > > >
> > > > That said, as far as I remember correctly, postmaster is generally not
> > > > supposed to touch shared memory (per the comments in postmaster.c),
> > > > so I'm not sure this approach is acceptable. On the other hand,
> > > > postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
> > > > so perhaps there's some precedent here.
> > > >
> > > IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
> > > may not be ideal, as it requires a spinlock to avoid races with the
> > > startup process and it is disallowed to take lock in postmaster main
> > > loop. Whereas, SlotSyncCtx->last_start_time doesn’t need a lock since
> > > the postmaster accesses it only when the worker is not alive.
> > >
> >
> > I agree.
>
> Could you clarify what issue might arise from checking
> SlotSyncCtx->stopSignaled without holding a spinlock in
> SlotSyncWorkerCanRestart()? Is it actually problematic?
>

We might not see issues in practice since stopSignaled changes only
once (false -> true), so value corruption is unlikely.
But, without a lock or memory barrier, correct value-read is not
guaranteed, e.g., on weakly ordered systems (like ARM64) the
postmaster may still see a stale value. This means the worker could be
restarted again, and the same unwanted log may still appear.

> That said, since the postmaster should generally avoid
> touching shared memory, it doesn't seem like a good idea
> for it to check SlotSyncCtx->stopSignaled. So I'm fine with
> instead lowering the log level for the "worker will not start"
> message to DEBUG1.
>

Okay, thanks. I'll share the updated patch soon.

--
Thanks,
Nisha

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-01T05:33:27Z

    On Tue, Mar 31, 2026 at 4:12 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Mar 31, 2026 at 11:35 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Mon, Mar 30, 2026 at 4:39 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Mon, Mar 30, 2026 at 1:18 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > > We were using the same log message in two places:
> > > > check_and_set_sync_info() and HandleSlotSyncMessage().
> > > > I think “will not start” fits better in the first case, while “will
> > > > stop” makes sense to keep in the second.
> > >
> > > Thanks for updating the patch!
> > >
> > > With the patch, in my testing, standby promotion always produces
> > > the following logs:
> > >
> > >     LOG:  replication slot synchronization worker will stop because
> > > promotion is triggered
> > >     LOG:  replication slot synchronization worker will not start
> > > because promotion was triggered
> > >
> > > It looks like the postmaster immediately restarts the slotsync worker after
> > > promotion terminates it, and that new worker then exits on seeing
> > > SlotSyncCtx->stopSignaled.
> > >
> > > IMO, always emitting both messages is a bit confusing. It would be nice to
> > > suppress the second one if possible.
> > >
> > > One idea would be to prevent the restart altogether. For example,
> > > ProcessSlotSyncMessage() could set SlotSyncCtx->last_start_time to
> > > a special value (like -1), and SlotSyncWorkerCanRestart() could return
> > > false (i.e., prevent postmater from starting up slotsync worker) when
> > > it sees that. Alternatively, SlotSyncWorkerCanRestart() could simply
> > > check SlotSyncCtx->stopSignaled.
> > >
> > > That said, as far as I remember correctly, postmaster is generally not
> > > supposed to touch shared memory (per the comments in postmaster.c),
> > > so I'm not sure this approach is acceptable. On the other hand,
> > > postmaster and the slotsync worker already rely on SlotSyncCtx->last_start_time,
> > > so perhaps there's some precedent here.
> > >
> > IIUC, checking SlotSyncCtx->stopSignaled in SlotSyncWorkerCanRestart()
> > may not be ideal, as it requires a spinlock to avoid races with the
> > startup process and it is disallowed to take lock in postmaster main
> > loop. Whereas, SlotSyncCtx->last_start_time doesn’t need a lock since
> > the postmaster accesses it only when the worker is not alive.
> >
>
> I agree.
>
> > Another option could be to log in check_and_set_sync_info() at DEBUG1
> > instead of LOG level. This message appears only after stopSignaled is
> > set, when promotion is already in progress and the first worker has
> > logged “will stop…”. The second worker doesn’t do any real work. Since
> > there’s nothing actionable for users, using DEBUG1 would keep it
> > useful for debugging (e.g., noticing immediate restarts) while
> > avoiding extra log noise. Thoughts?
> >
>
> +1.
>
> Do you think we can slightly tweak the comment in atop file to:
>
> On promotion the startup process sets 'stopSignaled' and uses this
> 'pid' to signal synchronizing process with PROCSIG_SLOTSYNC_MESSAGE
> and also to wake it up so that the process can immediately stop its
> synchronizing work. Setting 'stopSignaled' on the other hand is  used
> to handle the race condition....
>

Done.

> Also shall we quick exit ProcessSlotSyncMessage() if syncing is
> already finished by API?
>

Make sense. Fixed.

Please find the updated patch (v7) attached.

--
Thanks,
Nisha

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-01T05:33:51Z

    On Tue, Mar 31, 2026 at 3:56 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Tuesday, March 31, 2026 2:02 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> >
> > Please find the updated patch (v6) attached.
>
> Thanks for updating the patch. One minor comment:
>
> I think we could avoid interrupting and reporting an ERROR when
> IsSyncingReplicationSlots() returns false to avoid reporting ERROR unnecessarily
> when the slotsync has already finished.
>

Thanks for the review. Fixed above in v7.

--
Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-01T09:48:59Z

    On Wed, Apr 1, 2026 at 2:34 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Tue, Mar 31, 2026 at 3:56 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Tuesday, March 31, 2026 2:02 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > >
> > >
> > > Please find the updated patch (v6) attached.
> >
> > Thanks for updating the patch. One minor comment:
> >
> > I think we could avoid interrupting and reporting an ERROR when
> > IsSyncingReplicationSlots() returns false to avoid reporting ERROR unnecessarily
> > when the slotsync has already finished.
> >
>
> Thanks for the review. Fixed above in v7.

Thanks for updating the patch! It looks good to me, with just a few minor points
. If those are addressed, I'd like to push it.

+ * a new worker (or a new API call) that starts after the old worker was

"API" feels a bit vague. It might be clearer to explicitly say
"pg_sync_replication_slots()".

+ PROCSIG_SLOTSYNC_MESSAGE, /* ask slotsync worker/API to stop */

"API" here also feels a bit vague. So I'd like to use "ask slot synchronization
to stop" as the comment, instead.

+ * We cannot rely solely on 'stopSignaled' here because:
+ * 1) It resides in shared memory and is visible to all processes, so checking
+ *    it directly in ProcessInterrupts() would require additional checks to
+ *    ensure only the synchronizing process acts on it.
+ * 2) It has different lifetime semantics and cannot be reset after handling,
+ *    as it also guards against postmaster and promotion race conditions.
+ * 3) Accessing it requires acquiring a spinlock, which can be too expensive
+ *    or undesirable for every ProcessInterrupts() call.

Now that PROCSIG_SLOTSYNC_MESSAGE is in place, using SlotSyncShutdownPending
is intuitive. So it seems more useful to explain why stopSignaled is still
needed rather than why SlotSyncShutdownPending is used (i.e., why stopSignaled
is not sufficient). Since that rationale is already covered in the SlotSyncCtx
comments, I'd suggest removing this comment block.


As for backpatching, this looks like it should go back to v17, where slotsync
was introduced. Thought?

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-01T11:11:35Z

    On Wed, Apr 1, 2026 at 3:19 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Wed, Apr 1, 2026 at 2:34 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Tue, Mar 31, 2026 at 3:56 PM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Tuesday, March 31, 2026 2:02 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > >
> > > >
> > > > Please find the updated patch (v6) attached.
> > >
> > > Thanks for updating the patch. One minor comment:
> > >
> > > I think we could avoid interrupting and reporting an ERROR when
> > > IsSyncingReplicationSlots() returns false to avoid reporting ERROR unnecessarily
> > > when the slotsync has already finished.
> > >
> >
> > Thanks for the review. Fixed above in v7.
>
> Thanks for updating the patch! It looks good to me, with just a few minor points
> . If those are addressed, I'd like to push it.
>
> + * a new worker (or a new API call) that starts after the old worker was
>
> "API" feels a bit vague. It might be clearer to explicitly say
> "pg_sync_replication_slots()".
>

Done.

> + PROCSIG_SLOTSYNC_MESSAGE, /* ask slotsync worker/API to stop */
>
> "API" here also feels a bit vague. So I'd like to use "ask slot synchronization
> to stop" as the comment, instead.

Done.

> + * We cannot rely solely on 'stopSignaled' here because:
> + * 1) It resides in shared memory and is visible to all processes, so checking
> + *    it directly in ProcessInterrupts() would require additional checks to
> + *    ensure only the synchronizing process acts on it.
> + * 2) It has different lifetime semantics and cannot be reset after handling,
> + *    as it also guards against postmaster and promotion race conditions.
> + * 3) Accessing it requires acquiring a spinlock, which can be too expensive
> + *    or undesirable for every ProcessInterrupts() call.
>
> Now that PROCSIG_SLOTSYNC_MESSAGE is in place, using SlotSyncShutdownPending
> is intuitive. So it seems more useful to explain why stopSignaled is still
> needed rather than why SlotSyncShutdownPending is used (i.e., why stopSignaled
> is not sufficient). Since that rationale is already covered in the SlotSyncCtx
> comments, I'd suggest removing this comment block.

Okay, done.

>
> As for backpatching, this looks like it should go back to v17, where slotsync
> was introduced. Thought?

Right, the issue exists in v17 as well.

Attached the updated patch.

--
Thanks,
Nisha

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-02T05:01:37Z

    On Wed, Apr 1, 2026 at 8:11 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > As for backpatching, this looks like it should go back to v17, where slotsync
> > was introduced. Thought?
>
> Right, the issue exists in v17 as well.
>
> Attached the updated patch.

Thanks for updating the patch! LGTM.

I noticed that commit 1362bc33e02 updated the slotsync code so that a backend
performing slot synchronization is signaled on promotion, but it was applied
only to master. I’m not sure why it wasn’t backpatched to v17 and v18,
but it seems we need to backpatch it first before backpatching this patch.
Thought?

I've prepared patches for v18 and v17 and attached them. For the patch for
master, I only updated the commit message. For v18 and v17,
commit 1362bc33e02 needs to be applied first.

I haven't tested the v18 and v17 patches yet. I'll do that next.

Regards,

-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-04-02T06:34:18Z

    On Thu, Apr 2, 2026 at 10:31 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Wed, Apr 1, 2026 at 8:11 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > > As for backpatching, this looks like it should go back to v17, where slotsync
> > > was introduced. Thought?
> >
> > Right, the issue exists in v17 as well.
> >
> > Attached the updated patch.
>
> Thanks for updating the patch! LGTM.
>
> I noticed that commit 1362bc33e02 updated the slotsync code so that a backend
> performing slot synchronization is signaled on promotion, but it was applied
> only to master. I’m not sure why it wasn’t backpatched to v17 and v18,
>

It is because we added retry slot-sync logic for API in master in
commit 0d2d4a0ec3eca64e7f5ce7f7630b56a561b2663c. So, there is no
chance of API waiting except for the race condition being discussed
here.

> but it seems we need to backpatch it first before backpatching this patch.
> Thought?
>

I feel the use of API before this version was mainly for test-cases as
it was not production ready. So, it is less helpful to backpatch
1362bc33e02, if we want, we can backpatch only the worker part of the
fix. OTOH, as the issue is not frequent and we have some workaround
(at least for more common platforms) as well, we can consider not
backpatching it.

-- 
With Regards,
Amit Kapila.

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-02T13:16:16Z

    On Thu, Apr 2, 2026 at 3:34 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> It is because we added retry slot-sync logic for API in master in
> commit 0d2d4a0ec3eca64e7f5ce7f7630b56a561b2663c. So, there is no
> chance of API waiting except for the race condition being discussed
> here.

Thanks for the info!


> > but it seems we need to backpatch it first before backpatching this patch.
> > Thought?
> >
>
> I feel the use of API before this version was mainly for test-cases as
> it was not production ready. So, it is less helpful to backpatch
> 1362bc33e02, if we want, we can backpatch only the worker part of the
> fix. OTOH, as the issue is not frequent and we have some workaround
> (at least for more common platforms) as well, we can consider not
> backpatching it.

I see your point. OTOH, on second thought, if backpatching commit 1362bc33e02
along with this patch to v17 and v18 *is harmless*, I'd prefer to do so. Keeping
the slotsync shutdown code more consistent across versions would make future
backpatching easier, and selectively backpatching only parts of the shutdown
logic would be more complicated and error-prone. Thought?

Regards,

-- 
Fujii Masao

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-04-07T03:48:31Z

    On Thu, Apr 2, 2026 at 6:46 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Thu, Apr 2, 2026 at 3:34 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > I feel the use of API before this version was mainly for test-cases as
> > it was not production ready. So, it is less helpful to backpatch
> > 1362bc33e02, if we want, we can backpatch only the worker part of the
> > fix. OTOH, as the issue is not frequent and we have some workaround
> > (at least for more common platforms) as well, we can consider not
> > backpatching it.
>
> I see your point. OTOH, on second thought, if backpatching commit 1362bc33e02
> along with this patch to v17 and v18 *is harmless*, I'd prefer to do so.
>

As such I don't see a problem backpatching commit 1362bc33e02 as it
appears to be a localised change.

> Keeping
> the slotsync shutdown code more consistent across versions would make future
> backpatching easier, and selectively backpatching only parts of the shutdown
> logic would be more complicated and error-prone.
>

I agree with this line of reasoning here or in general as well but
personally I am a bit hesitant to back patch changes which are not
mandatory. In this particular case, I don't see any problem with
backpatching the part of code you want to backpatch, so I leave it to
your judgement.

-- 
With Regards,
Amit Kapila.

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-08T02:39:43Z

    On Tue, Apr 7, 2026 at 12:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> I agree with this line of reasoning here or in general as well but
> personally I am a bit hesitant to back patch changes which are not
> mandatory. In this particular case, I don't see any problem with
> backpatching the part of code you want to backpatch, so I leave it to
> your judgement.

Thanks for the comment!

I decided to backpatch commit 1362bc33e02. Although pg_sync_replication_slots()
lacks retry logic in v17 and v18 and is therefore less likely to block
promotion, the issue still exists in those versions.

Given that, it seemed worthwhile to backpatch the change and fix cases where
both the slotsync worker and pg_sync_replication_slots() can block promotion
when stuck in a wait.

I've pushed and backpatched the patch. Thanks!

Regards,

-- 
Fujii Masao

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-08T03:36:19Z

    On Wed, Apr 8, 2026 at 11:39 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Apr 7, 2026 at 12:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > I agree with this line of reasoning here or in general as well but
> > personally I am a bit hesitant to back patch changes which are not
> > mandatory. In this particular case, I don't see any problem with
> > backpatching the part of code you want to backpatch, so I leave it to
> > your judgement.
>
> Thanks for the comment!
>
> I decided to backpatch commit 1362bc33e02. Although pg_sync_replication_slots()
> lacks retry logic in v17 and v18 and is therefore less likely to block
> promotion, the issue still exists in those versions.
>
> Given that, it seemed worthwhile to backpatch the change and fix cases where
> both the slotsync worker and pg_sync_replication_slots() can block promotion
> when stuck in a wait.
>
> I've pushed and backpatched the patch. Thanks!

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?

Regards,

-- 
Fujii Masao

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-08T06:54:10Z

    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.

Regards,

-- 
Fujii Masao

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

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-08T09:45:10Z

    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

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

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-08T18:09:12Z

    On Wed, Apr 8, 2026 at 6:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> 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.

So the question is whether any extensions or third-party code depend on
NUM_PROCSIGNALS. I also couldn't find any such usage, so it seems safe from
an ABI perspective to change its value.


> 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?

Yes, you're right. So, in v17, I'll just move PROCSIG_SLOTSYNC_MESSAGE to
just before NUM_PROCSIGNALS.

Regards,

-- 
Fujii Masao