Multiple reporting of syslogger errors

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Multiple reporting of syslogger errors
Date: 2018-08-26 17:08:51
Message-ID: 338.1535303331@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed while testing the fix for Alexander Kukushkin's complaint that
if the syslogger process detects a problem (e.g., failure to open a new
log file) while log_destination is set to "csvlog", the error report
actually goes to THREE places. It gets spit out to csvlog, *and* to the
stderr log file, *and* to the original postmaster stderr. This seems
excessive.

The reason for the stderr log file output is that elog.c's
send_message_to_server_log has this hard-wired behavior:

/* If in the syslogger process, try to write messages direct to file */
if (am_syslogger)
write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);

The reason for the output to the original postmaster stderr is that
the stanza above that, beginning

/* Write to stderr, if enabled */
if ((Log_destination & LOG_DESTINATION_STDERR) || whereToSendOutput == DestDebug)

will always trigger in the syslogger because whereToSendOutput ==
DestDebug; we've forgotten to reset that to DestNone, as the postmaster
does immediately after forking the syslogger. Then, in the syslogger,
that stanza ends up in write_console() which writes to the inherited
stderr. (On the other hand, in Windows or other EXEC_BACKEND builds,
SubPostmasterMain will have done the reset, so that this doesn't occur
on those platforms.)

It seems to me to be a fairly clear bug that the syslogger process may
forget to set whereToSendOutput = DestNone depending on platform, so
I propose to fix that along with Kukushkin's problem.

However, I'm less sure about whether to change the hard-wired behavior
of writing syslogger errors to LOG_DESTINATION_STDERR regardless of
log_destination. It would seem more consistent to do that only if stderr
output is enabled in log_destination, but I wonder whether we made it work
like this intentionally. Or maybe it was intentional before we invented
csvlog, and nobody rethought it.

Comments?

regards, tom lane

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-08-26 18:50:09 Something's busted in plpgsql composite-variable handling
Previous Message Alexander Kukushkin 2018-08-26 16:00:23 Postmaster doesn't send SIGTERM to bgworker during fast shutdown when pmState == PM_STARTUP