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

Hi Shihao,

Thanks for testing it and for the review. v2 is attached, on today's
master. Point by point:

> 1. I think it kills more than client writes. With a 2s timeout it also
> killed
> a session waiting for sync rep, and a COPY FROM STDIN client that
> paused(in primary),

Reproduced: v1 terminated both after 2.02 s, and after 30.02 s with the
default; the sync rep one with "might not have been replicated to the
standby". Neither can absorb the barrier while it waits:
SyncRepWaitForLSN() doesn't run CHECK_FOR_INTERRUPTS(), and a COPY FROM
STDIN read only services ProcDiePending. On master, DROP DATABASE was
still waiting for them after 45 s and finished 15 ms after each one was
released.

v2 terminates only a client backend or WAL sender whose wait event is
ClientWrite. Any other straggler is left alone and checked again at
each wakeup. With v2, at 2 s and at 30 s, both sessions survive, DROP
waits as on master and finishes 4-15 ms after they move on, and the
five stuck cases from v1 still clear once the timeout expires. Both
are now negative cases in 061.

One gray zone is left: a session still sending a single large value to
a slow client also sits in ClientWrite, with no CHECK_FOR_INTERRUPTS()
until the value is out. Through a proxy limited to 2 MB/s, a 100 MB
value made DROP wait 50.5 s on master; v2 with the default 30 s
terminated that session at 30.05 s. I documented it in config.sgml,
but I don't see a way to tell it apart from a client that stopped
reading, with what another process can see. Ideas welcome.

> 2. 061 failed 4 of 11 runs here. slow_reader stops reading before the
> checksums launcher waits, so it gets killed.

Right: pg_disable_data_checksums() returns before the launcher emits
its barriers. 061 passed 11/11 here, but holding the launcher at
datachecksumsworker-launcher-delay until the reader had blocked, v1's
loop got the reader terminated 2/2. v2 keeps reading until
data_checksums is off: 0/2.

Pinned to one CPU, a second race showed up: woken by the barrier's
signal, a backend blocked in ClientWrite on a Unix socket can still fit
one more send (16 kB more queued while psql wasn't reading), reach a
CHECK_FOR_INTERRUPTS() and absorb, so the first DROP did not wait. v2's
061 wakes each stuck session with pg_log_backend_memory_contexts()
until it no longer logs. 061 then passed 11/11 normally and 11/11
pinned to one CPU. The mutants (no grace period, SIGINT, no kill, no
ClientWrite check) all fail it, without hanging now.

> 3. shouldTerminateBarrierStraggler() recheck pss_barrierGeneration
> under pss_mutex? because it can kill a new backend in the same slot.

Yes. Reproduced with gdb: with the waiter stopped on entry to
TerminateBarrierStraggler(), the stuck session exits and a new session,
also in ClientWrite, takes the same slot; v1 then terminated the new
one, 5/5. v2 reads pss_pid and pss_barrierGeneration together under
pss_mutex and skips the slot once the generation is reached, since a
new process starts at the current one: 0/5 (3/3 again with the recheck
removed). 061 can't reach that window; I can turn it into an injection
point test if you think it's worth it.

make check, the recovery suite (857 tests, including 061) and
test_checksums with checksum_extended pass on master f2edf5dd00d.

Regards,
Manu