Hi Gilles,
On Sat, 29 Oct 2016 22:00:08 +0200
Gilles Darold <gilles.darold@dalibo.com> wrote:
> The attached v10 of the current_logfiles patch
Attached is a patch to apply on top of the v10 patch.
It updates current_logfiles only once per log rotation.
I see no reason to open and write the file twice
if both csvlog and stderr logging is happening.
I have 2 more questions.
I don't understand why you're calling
logfile_writename(last_file_name, last_csv_file_name);
in the SIGHUP code. Presumably you've already
written the old logfile names to current_logfiles.
On SIGHUP you want to write the new names, not
the old ones. But the point of the SIGHUP code
is to look for changes in logfile writing and then
call logfile_rotate() to make those changes. And
logfile_rotate() calls logfile_writename() as appropriate.
Shouldn't the logfile_writename call in the SIGHUP
code be eliminated?
Second, and I've not paid really close attention here,
you're calling logfile_writename() with last_file_name
and last_csv_file_name in a number of places. Are you
sure that last_file_name and last_csv_file_name is reset
to the empty string if stderr/csvlog logging is turned
off and the config file re-read? You might be recording
that logging is going somewhere that's been turned off
by a config change. I've not noticed last_file_name and
(especially) last_csv_file_name getting reset to the empty
string.
FYI, ultimately, in order to re-try writes to current_logfiles
after ENFILE and EMFILE failure, I'm thinking that I'll probably
wind up with logfile_writename() taking no arguments. It will
always write last_file_name and last_csv_file_name. Then it's
a matter of making sure that these variables contain accurate
values. It would be helpful to let me know if you have any
insight regards config file re-read and resetting of these
variables before I dive into writing code which retries writes to
current_logfiles.
Regards,
Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
