Re: Add wait events for server logging destination writes

From: 신성준 <shinsj4653(at)gmail(dot)com>
To: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Kirk Wolak <wolakk(at)gmail(dot)com>
Cc: Nikolay Samokhvalov <nik(at)postgres(dot)ai>, assam258(at)gmail(dot)com, pgsql-hackers mailing list <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Andreas Karlsson <andreas(at)proxel(dot)se>, jian(dot)universality(at)gmail(dot)com
Subject: Re: Add wait events for server logging destination writes
Date: 2026-09-13 15:50:43
Message-ID: CACdN0M7iL+_z5=Ej44eLw1bqQi_p1hPscPDo8du-ZsE=kzbXzQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello.

v8 replaces an argument in 0001's commit message with a measurement,
after off-list review from Andrey Borodin and Kirk Wolak on the nesting
point Jakub raised.

On Mon, Aug 10, 2026 at 6:48 PM Jakub Wartak
<jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
> * but if something else is stuck in the
> some_important_stuff_that_may_also_hang() that gets lost because
> wait_event is going to be zero rather than WAIT_EVENT_BLAH and how do
> we find out?

I answered this in v7 by scanning for an ereport() reachable from
inside a wait event region, and said I could not find a region that
goes on to block afterwards. That was the wrong tool and I would
rather correct it here than leave the result in the archives.

Instead of reading the source I instrumented the mechanism: a
debug-only stack that pushes on pgstat_report_wait_start() and pops on
pgstat_report_wait_end(), with the depth capped at one so that opening
a region inside another one fails an assertion. Running the test
suite that way, the only log write that turns up inside another region
is one the scan could not have found:

PostgresMain -> pq_getbyte -> pq_recvbuf -> secure_read
-> WaitEventSetWait [ClientRead published]
-> SIGQUIT -> quickdie
-> ereport(WARNING, "terminating connection because of
unexpected SIGQUIT signal")
-> send_message_to_server_log
-> write_console [StderrWrite]

quickdie() reports from a signal handler, so it can land inside
whatever region the backend happens to be sitting in. A signal
handler is not in anyone's call graph, which is why counting callees
the way I did in v7 was never going to settle this.

It is specifically the PMQUIT_NOT_SENT branch that gets there, that
is, a SIGQUIT sent to the backend directly rather than by the
postmaster. The two postmaster-initiated branches report at
WARNING_CLIENT_ONLY, which never reaches the server log at all.

It does not change the conclusion for these events, but it does change
the reason. quickdie() calls _exit(2) immediately afterwards, so the
masked ClientRead is never read back by anything. v8's commit message
says that, and drops the claim that the regions in the tree wrap a
single call and then end.

Two caveats on the measurement. The assertion aborts at the first
nested region, so this finds the first case per process rather than
all of them; and it only covers what the suite exercises.

On the general fix: the follow-up I promised in v7 now exists as a
patch. It is the check above, plus one thing the check forced into
the open -- "close the region I opened" and "reset the field after a
longjmp out of a region that may not have been open" are both spelled
pgstat_report_wait_end() today, and a stack cannot treat them the
same. Ten call sites are the second kind (AbortTransaction,
AbortSubTransaction, ShutdownAuxiliaryProcess, WalSndErrorCleanup and
the sigsetjmp blocks of the aux processes). As I said in v7 I will
put that on its own thread rather than fold it in here.

v8 is attached. There is no code change from v7; the only difference
is the commit message paragraph described above, plus a rebase onto
current master (8c7a74c3239).

v8-0001 - portable part (SysloggerWrite, StderrWrite, SyslogWrite)
v8-0002 - Windows part (WriteConsoleW plus EventlogWrite)

I checked it by:

- git am of both patches onto 8c7a74c3239, clean;
- full Meson build (cassert, debug) -- no warnings;
- 0001 building on its own;
- regression, isolation and TAP suites green: 361 tests, 0 failures.
40 are skipped here, none of them for a reason this patch affects:
17 behind PG_TEST_EXTRA, 16 wanting an injection-points build, 5
expensive checksum tests, plus ICU and SSPI.

Regards,
Seongjun Shin

Attachment Content-Type Size
v8-0001-Add-wait-events-for-server-logging-destination-wr.patch application/octet-stream 10.0 KB
v8-0002-Add-wait-events-for-Windows-specific-logging-outp.patch application/octet-stream 4.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Rustam ALLAKOV 2026-09-13 16:12:38 Re: Fold NOT IN / <> ALL expressions containing NULL to FALSE
Previous Message vignesh C 2026-09-13 15:30:53 Re: Distinguish publication exclusions in object addresses