Re: [PG19] Promotion hangs behind a backend blocked on a stuck client

Hi,

shihao zhong wrote:
> Manu, do you want to post it? If not, I am happy to take it.

I would, thanks. Attached is v1 of (2) for master, for review.

After procsignal_barrier_timeout, WaitForProcSignalBarrier() sends
SIGTERM to any client backend or WAL sender that still has not absorbed
the barrier, as pg_terminate_backend() would, and keeps waiting until
it exits. Other processes are never signaled. The waiter logs

LOG: terminating process 12345 because it has not accepted
ProcSignalBarrier within 2000 ms

and the session ends with the usual "terminating connection due to
administrator command".

Choices I made, all easy to change:

- SIGTERM rather than SignalRecoveryConflict(). A backend blocked in a
client write only services ProcDiePending
(ProcessClientWriteInterrupt()), which is why the conflict variant of
the prototype released none of the 12 stuck cases.

- It applies to every waiter, not only the startup process (marked
XXX in the patch). The same hang happens on a primary; in the test
the waiters are a client backend (DROP DATABASE), the datachecksums
launcher and the startup process.

- A GUC: default 30s, the same as max_standby_streaming_delay, 0
disables, PGC_SIGHUP since it decides whether other users' sessions
get terminated. Fixed time vs GUC, name and default are all open.

- Signaled once per process; after that the wait continues as today
and wakes up when the process exits.

- No dedicated message for the terminated session. Its client cannot
receive it anyway, since a blocked write switches output to DestNone;
the waiter's LOG line names the PID.

The new TAP test (recovery/061) uses a psql whose output is never read,
so its backend blocks in ClientWrite, and a 2 s timeout. On unpatched
master, with the same setup in a separate script, each of the cases
below was still stuck after 10 s and finished a few ms after a manual
pg_terminate_backend(). With the patch, over 5 runs of the test:

- DROP DATABASE with the session on another database
- pg_enable_data_checksums()
- DROP DATABASE with a WAL sender stuck in a base backup
- replay of the record enabling logical decoding on a standby
- promotion of a standby with a logical slot

all completed, the stuck session terminated 2001-2020 ms after the
operation started, never before the timeout. Negative control: an idle
session, one in pg_sleep() and a slow but steady reader survived
DROP DATABASE and pg_disable_data_checksums() in all runs, with no
termination and the DROP under 2 s; an idle session on the standby
survived the promotion. With the timeout at 0, DROP DATABASE kept
waiting, as on master.

make check, the recovery suite (848 tests, with injection points) and
test_checksums with checksum_extended (458) pass; no other test
triggered a termination.

What it does not cover:

- Recovery conflicts. With the patch, a snapshot conflict against such
a session still stopped replay for the whole 15 s window (3/3): that
path signals with SignalRecoveryConflict() and is not changed here.
Escalating to SIGTERM after max_standby_*_delay for a backend in a
client write could be a follow-up, if people want it.

- A backend stuck while sending an ERROR, which holds interrupts; as
reported upthread, pg_terminate_backend() does not release it
either.

On v6: (2) also covers replay of the enable record, data checksums,
DROP DATABASE and WAL senders, which v6 does not. v6 is still the
cheaper path at end of recovery: without it a promotion behind a stuck
session waits the full timeout (30 s by default) and terminates that
session, even though in the failover-slot case I checked, that backend
already had the up-to-date status. So I see v6 as a fast path rather
than something (2) makes unnecessary, but that is your call.

Regards,
Manu