Refactor fallback to stderr for csvlog to handle better WIN32 service case

send_message_to_server_log() would force a redirection of a log entry to
stderr in some cases for csvlog, like the syslogger not being available
yet.  If this happens, csvlog would fall back to stderr to log
some information rather than nothing.  The code was organized so as
stderr is done before csvlog, with csvlog checking that stderr did not
happen yet with a reversed condition.  With this code organization, it
could be possible to lose some messages if running Postgres as a service
on WIN32, as there is no usable stderr, and the handling of the
StringInfoData holding the message for stderr was rather confusing
because of that.

This commit moves the csvlog handling to be before stderr, as as we are
able to track down if it is necessary to log something to stderr.  The
reduces the handling of stderr to be in a single code path, adding a
fallback to event logs for a WIN32 service.  This also simplifies the
way we handle the StringInfoData for stderr, making easier the
integration of new file-based log destinations.  I got to play with
services and event logs on Windows while checking this change.

Reviewed-by: Chris Bandy
Discussion: https://postgr.es/m/YV0vwBovEKf1WXkl@paquier.xyz
This commit is contained in:
Michael Paquier 2021-10-08 11:08:35 +09:00
parent 08e2daf06c
commit 8b76f89c37
1 changed files with 24 additions and 30 deletions

View File

@ -3008,6 +3008,7 @@ static void
send_message_to_server_log(ErrorData *edata)
{
StringInfoData buf;
bool fallback_to_stderr = false;
initStringInfo(&buf);
@ -3159,8 +3160,27 @@ send_message_to_server_log(ErrorData *edata)
}
#endif /* WIN32 */
/* Write to stderr, if enabled */
if ((Log_destination & LOG_DESTINATION_STDERR) || whereToSendOutput == DestDebug)
/* Write to csvlog, if enabled */
if (Log_destination & LOG_DESTINATION_CSVLOG)
{
/*
* Send CSV data if it's safe to do so (syslogger doesn't need the
* pipe). If this is not possible, fallback to an entry written to
* stderr.
*/
if (redirection_done || MyBackendType == B_LOGGER)
write_csvlog(edata);
else
fallback_to_stderr = true;
}
/*
* Write to stderr, if enabled or if required because of a previous
* limitation.
*/
if ((Log_destination & LOG_DESTINATION_STDERR) ||
whereToSendOutput == DestDebug ||
fallback_to_stderr)
{
/*
* Use the chunking protocol if we know the syslogger should be
@ -3189,34 +3209,8 @@ send_message_to_server_log(ErrorData *edata)
if (MyBackendType == B_LOGGER)
write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);
/* Write to CSV log if enabled */
if (Log_destination & LOG_DESTINATION_CSVLOG)
{
if (redirection_done || MyBackendType == B_LOGGER)
{
/*
* send CSV data if it's safe to do so (syslogger doesn't need the
* pipe). First get back the space in the message buffer.
*/
pfree(buf.data);
write_csvlog(edata);
}
else
{
/*
* syslogger not up (yet), so just dump the message to stderr,
* unless we already did so above.
*/
if (!(Log_destination & LOG_DESTINATION_STDERR) &&
whereToSendOutput != DestDebug)
write_console(buf.data, buf.len);
pfree(buf.data);
}
}
else
{
pfree(buf.data);
}
/* No more need of the message formatted for stderr */
pfree(buf.data);
}
/*