Le 31/10/2016 à 04:35, Karl O. Pinc a écrit : > Hi Gilles, > > On Sat, 29 Oct 2016 22:00:08 +0200 > Gilles Darold <gilles.dar...@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 do not take the effort to do that because log rotation is not something that occurs too often and I don't want to change the conditional "time_based_rotation" code lines in logfile_rotate() for readability. My though was also that on high load, log rotation is automatically disabled so logfile_writename() is not called and there will be no additional I/O. But why not, if commiters want to save this additional I/O, this patch can be applied. > 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? No, when you change the log_destination and reload configuration you have to refresh the content of current_logfiles, in this case no new rotation have been done and you have to write last_file_name and/or last_csv_file_name. > 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. In the patch I do not take care if the value of last_file_name and last_csv_file_name have been reseted or not because following the log_destination they are written or not to current_logfiles. When they are written, they always contain the current log filename because the call to logfile_writename() always appears after their assignment to the new rotated filename. > 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. As explain above, last_file_name and last_csv_file_name always contains the last log file name, then even in case of logfile_writename() repeated failure. Those variables might have been updated in case of log rotation occurs before a new call to logfile_writename(). In case of ENFILE and EMFILE failure, log rotation is disabled and logfile_writename() not called, last_file_name and last_csv_file_name will still contain the last log file name. -- Gilles Darold Consultant PostgreSQL http://dalibo.com - http://dalibo.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
