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 <[email protected]> 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
v8-0001-Add-wait-events-for-server-logging-destination-wr.patch
Description: Binary data
v8-0002-Add-wait-events-for-Windows-specific-logging-outp.patch
Description: Binary data
