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

Reply via email to