From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Lost logs with csvlog redirected to stderr under WIN32 service |
Date: | 2021-10-06 05:10:24 |
Message-ID: | YV0vwBovEKf1WXkl@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi all,
While reviewing the code of elog.c to plug in JSON as a file-based log
destination, I have found what looks like a bug in
send_message_to_server_log(). If LOG_DESTINATION_CSVLOG is enabled,
we would do the following to make sure that the log entry is not
missed:
- If redirection_done or in the logging collector, call write_csvlog()
to write the CSV entry using the piped protocol or write directly if
the logging collector does the call.
- If the log redirection is not available yet, we'd just call
write_console() to redirect the message to stderr, which would be done
if it was not done in the code block for stderr before handling CSV to
avoid duplicates. This uses a condition that matches the one based on
Log_destination and whereToSendOutput.
Now, in the stderr code path, we would actually do more than that:
- write_pipe_chunks() for a non-syslogger process if redirection is
done.
- If there is no redirection, redirect to eventlog when running as a
service on WIN32, or simply stderr with write_console().
So at the end, if one enables only csvlog, we would not capture any
logs if the redirection is not ready yet on WIN32 when running as a
service, meaning that we could lose some precious information if there
is for example a startup failure.
This choice comes from fd801f4 in 2007, that introduced csvlog as
a log_destination.
I think that there is a good argument for back-patching a fix, but I
don't recall seeing anybody complaining about that and I just need
that for the business with JSON. I have thought about various ways to
fix that, and finished with a solution where we handle csvlog first,
and fallback to stderr after so as there is only one code path for
stderr, as of the attached. This reduces a bit the confusion around
the handling of the stderr data that gets free()'d in more code paths
than really needed.
Thoughts or objections?
--
Michael
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-fallback-to-stderr-when-using-only-csvlog.patch | text/x-diff | 3.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jaime Casanova | 2021-10-06 05:21:02 | wrapping CF 2021-09 |
Previous Message | Andres Freund | 2021-10-06 04:54:37 | Re: Adding CI to our tree |