Le 04/11/2016 à 14:17, Karl O. Pinc a écrit : > On Mon, 31 Oct 2016 10:19:18 +0100 > Gilles Darold <gilles.dar...@dalibo.com> wrote: > >> Le 31/10/2016 à 04:35, Karl O. Pinc a écrit : >>> 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. > Ok. You didn't put this into your v11 patch so it can be submitted to > the committers as a separate patch. > >>> 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. > I don't understand. Please explain what's wrong with the > picture I have of how logging operates on receipt of SIGHUP. > Here is my understanding: > > The system is running normally, current_logfiles exists and contains > the log file paths of the logs presently being written into. These > paths end with the file names in last_file_name and/or > last_csv_file_name. (I'm assuming throughout my description here that > log_destination is writing to the file system at all.) Yes, also if log_collector is on and log_destination is not stderr or csvlog, current_logfiles exists too but it is emtpy. When log_collector is off this file doesn't exists.
> A SIGHUP arrives. The signal handler, sigHupHandler(), sets the > got_SIGHUP flag. Nothing else happens. > > The logging process wakes up due to the signal. > Either there's also log data or there's not. If there's > not: > > The logging process goes through it's processing loop and finds, > at line 305 of syslogger.c, got_SIGHUP to be true. > Then it proceeds to do a bunch of assignment statements. > If it finds that the log directory or logfile name changed > it requests immediate log file rotation. It does this by > setting the request_rotation flag. If log_destination > or logging_collector changed request_rotation is not set. > > Then, your patch adds a call to logfile_writename(). > But nothing has touched the last_file_name or last_csv_file_name > variables. You rewrite into current_logfiles what's > already in current_logfiles. Your picture is good, you just miss the case where we just change log_destination. In this case, syslogger add/change log file destination but rotation_requested is false. If write to current_logfiles is conditional to this flag, it will never reflect the file change until next rotation, see why next answer bellow. If log_destination remain unchanged I agree that I am rewriting the same information, but I don't think that this kind of sporadic I/O is enough to append code to store the old log_destination value and check if there is a change like what is done with Log_directory. This is my opinion, but may be I'm wrong to consider that those isolated and not critical I/O doesn't justify this work. > If there is log data in the pipe on SIGHUP > and it's csv log data then if there's a csv > log file open that's the file that's written to. > last_csv_file_name does not change so current_logfiles > does not need to be re-written. If there is no csv > log file open then open_csvlogfile() is called > and it calls logfile_writename(). No need to > call logfile_writename() again when processing the > SIGHUP. Yes, but the problem here is that logfile_writename() doesn't write the change because the test (Log_destination & LOG_DESTINATION_CSVLOG) returns false, Log_destination is set after the file is successfully created. This make me though that the call of logfile_writename() into function open_csvlogfile() can be removed, thanks for pointing me that. I attached a v12 patch with some comment fix in the call to logfile_writename(). Regards, -- Gilles Darold Consultant PostgreSQL http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index adab2f8..645cbb9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4143,6 +4143,12 @@ SELECT * FROM parent WHERE key = 2400; <primary>server log</primary> </indexterm> + <para>When logs are written to the file-system their paths, names, and + types are recorded in + the <xref linkend="storage-pgdata-current-logfiles"> file. This provides + a convenient way to find and access log content without establishing a + database connection.</para> + <sect2 id="runtime-config-logging-where"> <title>Where To Log</title> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 2e64cc4..f5bfe59 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -15443,6 +15443,19 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n); </row> <row> + <entry><literal><function>pg_current_logfile()</function></literal></entry> + <entry><type>text</type></entry> + <entry>primary log file name in use by the logging collector</entry> + </row> + + <row> + <entry><literal><function>pg_current_logfile(<type>text</>)</function></literal></entry> + <entry><type>text</type></entry> + <entry>log file name, of log in the requested format, in use by the + logging collector</entry> + </row> + + <row> <entry><literal><function>pg_my_temp_schema()</function></literal></entry> <entry><type>oid</type></entry> <entry>OID of session's temporary schema, or 0 if none</entry> @@ -15660,6 +15673,34 @@ SET search_path TO <replaceable>schema</> <optional>, <replaceable>schema</>, .. the time when the postmaster process re-read the configuration files.) </para> + <indexterm> + <primary>pg_current_logile</primary> + </indexterm> + + <indexterm> + <primary>Logging</primary> + <secondary>pg_current_logfile function</secondary> + </indexterm> + + <para> + <function>pg_current_logfile</function> returns, as <type>text</type>, + the path of either the csv or stderr log file currently in use by the + logging collector. This is a path including the + <xref linkend="guc-log-directory"> directory and the log file name. + Log collection must be active or the return value + is <literal>NULL</literal>. When multiple logfiles exist, each in a + different format, <function>pg_current_logfile</function> called + without arguments returns the path of the file having the first format + found in the ordered + list: <systemitem>stderr</>, <systemitem>csvlog</>. + <literal>NULL</literal> is returned when no log file has any of these + formats. To request a specific file format supply, + as <type>text</type>, either <systemitem>csvlog</> + or <systemitem>stderr</> as the value of the optional parameter. The + return value is <literal>NULL</literal> when the log format requested + is not a configured <xref linkend="guc-log-destination">. + </para> + <indexterm> <primary>pg_my_temp_schema</primary> </indexterm> diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index fddb69b..1cfb9da 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -60,6 +60,28 @@ Item <entry>Subdirectory containing per-database subdirectories</entry> </row> +<row id="storage-pgdata-current-logfiles" xreflabel="current_logfiles"> + <entry> + <indexterm> + <primary><filename>current_logfiles</filename></primary> + </indexterm> + <indexterm> + <primary>Logging</primary> + <secondary><filename>current_logfiles</filename> file</secondary> + </indexterm> + <filename>current_logfiles</> + </entry> + <entry> + A file recording the current log file(s) used by the syslogger and + its log format, <systemitem>stderr</> or <systemitem>csvlog</>. Each line + of the file is a space separated list of two elements: the log format and + the full path to the log file including the value + of <xref linkend="guc-log-directory">. The log format must be present + in <xref linkend="guc-log-destination"> to be listed in the file. This file + is present only when <xref linkend="guc-logging-collector"> is + activated.</entry> +</row> + <row> <entry><filename>global</></entry> <entry>Subdirectory containing cluster-wide tables, such as diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index fd62d66..4e121fa 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -146,6 +146,7 @@ static char *logfile_getname(pg_time_t timestamp, const char *suffix); static void set_next_rotation_time(void); static void sigHupHandler(SIGNAL_ARGS); static void sigUsr1Handler(SIGNAL_ARGS); +static void logfile_writename(char *filename, char *csvfilename); /* @@ -348,6 +349,13 @@ SysLoggerMain(int argc, char *argv[]) rotation_disabled = false; rotation_requested = true; } + + /* + * Force rewriting last log filename when reloading configuration, + * even if rotation_requested is false, log_destination may have + * been changed and we don't want to wait the next file rotation. + */ + logfile_writename(last_file_name, last_csv_file_name); } if (Log_RotationAge > 0 && !rotation_disabled) @@ -513,8 +521,13 @@ SysLogger_Start(void) pid_t sysloggerPid; char *filename; - if (!Logging_collector) + if (!Logging_collector) { + /* Logging collector is not enabled. We don't know where messages are + * logged. Remove outdated file holding the current log filenames. + */ + unlink(CURRENT_LOG_FILENAME); return 0; + } /* * If first time through, create the pipe which will receive stderr @@ -574,6 +587,8 @@ SysLogger_Start(void) syslogFile = logfile_open(filename, "a", false); + logfile_writename(filename, NULL); + pfree(filename); #ifdef EXEC_BACKEND @@ -1209,6 +1224,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for) return; } + logfile_writename(filename, csvfilename); + fclose(syslogFile); syslogFile = fh; @@ -1220,7 +1237,6 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for) } /* Same as above, but for csv file. */ - if (csvlogFile != NULL && (time_based_rotation || (size_rotation_for & LOG_DESTINATION_CSVLOG))) { @@ -1253,6 +1269,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for) return; } + logfile_writename(last_file_name, csvfilename); + fclose(csvlogFile); csvlogFile = fh; @@ -1365,3 +1383,57 @@ sigUsr1Handler(SIGNAL_ARGS) errno = save_errno; } + +/* + * Store the name of the file(s) where the log collector, when enabled, writes + * log messages. Useful for finding the name(s) of the current log file(s) + * when there is time-based logfile rotation. Filenames are stored in a + * temporary file and renamed into the final destination for atomicity. + */ +static void +logfile_writename(char *filename, char *csvfilename) +{ + FILE *fh; + char tempfn[MAXPGPATH]; + + snprintf(tempfn, sizeof(tempfn), "%s", + CURRENT_LOG_FILENAME); + strcat(tempfn, ".tmp"); + if ((fh = logfile_open(tempfn, "w", true) ) == NULL) + { + return; + } + if (filename && (Log_destination & LOG_DESTINATION_STDERR)) { + if (fprintf(fh, "stderr %s\n", filename) < 0) + { + ereport(LOG, + (errcode_for_file_access(), + errmsg("could not write stderr log file path \"%s\": %m", + tempfn))); + fclose(fh); + return; + } + } + + if (csvfilename && (Log_destination & LOG_DESTINATION_CSVLOG)) { + if (fprintf(fh, "csvlog %s\n", csvfilename) < 0) + { + ereport(LOG, + (errcode_for_file_access(), + errmsg("could not write csvlog log file path \"%s\": %m", + tempfn))); + fclose(fh); + return; + } + } + fclose(fh); + + if (rename(tempfn, CURRENT_LOG_FILENAME) != 0) + { + ereport(LOG, + (errcode_for_file_access(), + errmsg("could not rename file \"%s\": %m", + tempfn))); + return; + } +} diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c index 0da051a..06acdb2 100644 --- a/src/backend/utils/adt/misc.c +++ b/src/backend/utils/adt/misc.c @@ -19,6 +19,7 @@ #include <dirent.h> #include <math.h> #include <unistd.h> +#include <sys/stat.h> #include "access/sysattr.h" #include "catalog/pg_authid.h" @@ -892,3 +893,122 @@ parse_ident(PG_FUNCTION_ARGS) PG_RETURN_DATUM(makeArrayResult(astate, CurrentMemoryContext)); } + +/* + * Report current log file used by log collector + */ +Datum +pg_current_logfile(PG_FUNCTION_ARGS) +{ + FILE *fd; + char lbuffer[MAXPGPATH]; + text *fmt; + char *logfmt; + struct stat stat_buf; + char *log_format; + char *log_filename; + + /* The log format parameter is optional */ + if (PG_NARGS() == 1) { + fmt = PG_ARGISNULL(0) ? NULL : PG_GETARG_TEXT_PP(0); + if (fmt != NULL) { + logfmt = text_to_cstring(fmt); + if ( (strcmp(logfmt, "stderr") != 0) && + (strcmp(logfmt, "csvlog") != 0) ) { + ereport(ERROR, + (errmsg("log format %s not supported, possible values are stderr or csvlog", logfmt))); + PG_RETURN_NULL(); + } + } + } else { + logfmt = NULL; + } + + if (!Logging_collector) + PG_RETURN_NULL(); + + /* Check if current log file is present */ + if (stat(CURRENT_LOG_FILENAME, &stat_buf) != 0) + PG_RETURN_NULL(); + + fd = AllocateFile(CURRENT_LOG_FILENAME, "r"); + if (fd == NULL) + { + if (errno != ENOENT) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", + CURRENT_LOG_FILENAME))); + PG_RETURN_NULL(); + } + + /* + * Read the file to gather current log filename(s) registered + * by the syslogger. + */ + while (fgets(lbuffer, sizeof(lbuffer), fd) != NULL) { + + /* Check for a read error. */ + if (ferror(fd)) { + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not read file \"%s\": %m", CURRENT_LOG_FILENAME))); + FreeFile(fd); + break; + } + + /* remove trailing newline */ + if (strchr(lbuffer, '\n') != NULL) + *strchr(lbuffer, '\n') = '\0'; + + /* extract log format and log file path from the line */ + log_format = strtok(lbuffer, " "); + if (log_format) + { + log_filename = strtok(NULL, " "); + if (!log_filename) { + ereport(ERROR, + (errmsg("unexpected line format in file %s", CURRENT_LOG_FILENAME))); + break; + } + } + else + { + ereport(ERROR, + (errmsg("unexpected line format in file %s", CURRENT_LOG_FILENAME))); + break; + } + + /* + * When no log format is provided as argument always reports + * the first registered log file in CURRENT_LOG_FILENAME. + */ + if (logfmt == NULL) + break; + + /* report the entry corresponding to the requested format */ + if (strcmp(logfmt, log_format) == 0) + break; + } + /* Close the current log filename file. */ + if (FreeFile(fd)) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not close file \"%s\": %m", CURRENT_LOG_FILENAME))); + + + if (log_filename[0] == '\0') + PG_RETURN_NULL(); + + /* Return null when csvlog is requested but we have a stderr log */ + if ( (logfmt != NULL) && (strcmp(logfmt, "csvlog") == 0) + && !(Log_destination & LOG_DESTINATION_CSVLOG) ) + PG_RETURN_NULL(); + + /* Return null when stderr is requested but we have a csv log */ + if ( (logfmt != NULL) && (strcmp(logfmt, "stderr") == 0) + && !(Log_destination & LOG_DESTINATION_STDERR) ) + PG_RETURN_NULL(); + + PG_RETURN_TEXT_P(cstring_to_text(log_filename)); +} diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 17ec71d..78e23a6 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -3179,6 +3179,8 @@ DATA(insert OID = 2621 ( pg_reload_conf PGNSP PGUID 12 1 0 0 0 f f f f t f v s DESCR("reload configuration files"); DATA(insert OID = 2622 ( pg_rotate_logfile PGNSP PGUID 12 1 0 0 0 f f f f t f v s 0 0 16 "" _null_ _null_ _null_ _null_ _null_ pg_rotate_logfile _null_ _null_ _null_ )); DESCR("rotate log file"); +DATA(insert OID = 3800 ( pg_current_logfile PGNSP PGUID 12 1 0 0 0 f f f f f f v s 1 0 25 "25" _null_ _null_ _null_ _null_ _null_ pg_current_logfile _null_ _null_ _null_ )); +DESCR("current logging collector file location"); DATA(insert OID = 2623 ( pg_stat_file PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 2249 "25" "{25,20,1184,1184,1184,1184,16}" "{i,o,o,o,o,o,o}" "{filename,size,access,modification,change,creation,isdir}" _null_ _null_ pg_stat_file_1arg _null_ _null_ _null_ )); DESCR("get information about file"); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 78545da..2faefdd 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -466,4 +466,12 @@ extern bool has_rolreplication(Oid roleid); extern bool BackupInProgress(void); extern void CancelBackup(void); +/* in backend/utils/adt/misc.c and backend/postmaster/syslogger.c */ +/* + * Name of file where current log messages are written when log collector is + * enabled. Useful to find the name of the current log file when a time-based + * rotation is defined. + */ +#define CURRENT_LOG_FILENAME "current_logfiles" + #endif /* MISCADMIN_H */ diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h index 90f5132..df9a1e1 100644 --- a/src/include/utils/builtins.h +++ b/src/include/utils/builtins.h @@ -522,6 +522,7 @@ extern Datum pg_collation_for(PG_FUNCTION_ARGS); extern Datum pg_relation_is_updatable(PG_FUNCTION_ARGS); extern Datum pg_column_is_updatable(PG_FUNCTION_ARGS); extern Datum parse_ident(PG_FUNCTION_ARGS); +extern Datum pg_current_logfile(PG_FUNCTION_ARGS); /* oid.c */ extern Datum oidin(PG_FUNCTION_ARGS);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers