On Fri, Jul 17, 2026 at 5:55 PM 신성준 <[email protected]> wrote:
[..]
> v6-0001 - portable part (SysloggerWrite, StderrWrite, SyslogWrite, now
> including the openlog() call)
> v6-0002 - Windows part (WriteConsoleW plus EventlogWrite)
>
Hi Seongjun,
I think it's worthy addition as functionality and the code is basically just
wrapping pgstat_report_wait_start/pgstat_report_wait_endt(). I've seen at
least one case of bank being taken down due to similiar issues (rsyslogd stuck
due to stuck sync TCP remote connection, backpropagating to backends), AFAIR
wait_events were NULL (so useless), but strack-trace collection showed
processing stuck on syslog(). I've tested this using:
1) LD_PRELOAD for syslog() with sleep(1s) and started server that way
2) log_connections=on
3) and pgbench -c N -j N --connect so basically each new connection was stuck on
this, and in the pg_stat_active I could see this new "SyslogWrite" event, so
+1 from me the functionality.
As for review:
a. I have no idea why this is split into two patches, it could be just one to
make things easier to process?
b. if we have nested use, let's say:
pgstat_report_wait_start(WAIT_EVENT_BLAH);
something()
while(work) {
ereport(LOG, ..) {
NEW: pgstat_report_wait_start(WAIT_EVENT_SYSLOG_WRITE);
syslog(..); // or some other write(2) to log
NEW: pgstat_report_wait_end(); // wait_event=0
}
some_important_stuff_that_may_also_hang();
}
pgstat_report_wait_wait_end();
this "NEW" code-path is going to zero-out WAIT_EVENT_BLAH:
* if it get stuck SYSLOG_WRITE and learn this that way, great, but ..
* 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?
The other reviewer (Jihyun) in the parallel subthread I thnink also mentioned
this danger when discussing AddToDataDirLockFile(), but I'm not sure if I
understood him fully, but it has this pattern of wait_event_start(),
pg_fsync() which has nested wait_event_start for fsync , then _end(), and then
yet another _end(), but it seems to be the only place like that (?)
What about extensions using ereport() ? No idea..
Probably correct way to address would be to find a way for _end() to bring
back saved prior wait event, so perhaps it should not just _end() to 0, but
return it back to the previous one if that was set (I'm assuming this is
in code paths never being in hot-paths so simple if() shouldn't matter), but
I'm not 100% sure if that's good way, maybe others can express their opinion
on this one (we would require new conditional
pgstat_report_wait_start($previous_wait_event) if wait_event !=0 in this
patchset if we choose that route).
-J.
