Le 26/10/2016 à 05:30, Karl O. Pinc a écrit :
> On Tue, 18 Oct 2016 14:18:36 +0200
> Gilles Darold <gilles.dar...@dalibo.com> wrote:
>
>> Here is the v6 of the patch, here is the description of the
>> pg_current_logfile() function, I have tried to keep thing as simple as
>> possible:
>>
>> pg_current_logfile( [ destination text ] )
>> -----------------------------------------------------
> Attached is a doc patch to apply on top of your v6 patch.

Thanks a lot for the documentation fixes, I've also patched some of your
changes, see v7 of the patch and explanations bellow.

> Changes are, roughly:
>
> Put pg_log_file in alphabetical order in the file name listing
> and section body.

This file is now named current_logfile, I have changed the named in the
documentation, especially in storage.sgml. Sorry for missing that in my
v6 patch.

One other change in documentation is the explanation of values stored in
this file. This is a path: log_directory/log_filename, and no more the
log file name only. This will help to get full path of the log at system
command level. This is also the value returned by function the
pg_current_logfile() to be able to read file directly with a simple:

     SELECT pg_read_file(pg_current_logfile());

It can be also useful if the file is included in a backup to find the
last log corresponding to the backup.


> I also have a couple of preliminary comments.
>
> It seems like the code is testing for whether the
> logfile is a CSV file by looking for a '.csv' suffix
> on the end of the file name.  This seems a little fragile.
> Shouldn't it be looking at something set internally when the
> log_destination config declaration is parsed?

Right, even if syslogger.c always adds the .csv suffix to the log file.
This method would failed if the log file was using this extension even
for a stderr format. It has been fixed in the v7 patch to no more use
the extension suffix.

> Since pg_log_file may contain only one line, and that
> line may be either the filename of the csv log file or
> the file name of the stderr file name it's impossible
> to tell whether that single file is in csv or stderr
> format.  I suppose it might be possible based on file
> name suffix, but Unix expressly ignores file name
> extensions and it seems unwise to force dependence
> on them.  Perhaps each line could begin with
> the "type" of the file, either 'csv' or 'stderr'
> followed by a space and the file name? 

The current_logfile may contain 2 lines, one for csvlog and an other for
stderr when they are both defined in log_destination. As said above, the
csvlog file will always have the .csv suffix, I guess it is enough to
now the format but I agree that it will not be true in all cases. To
keep things simple I prefer to only register the path to the log file,
external tools can easily detect the format or can ask for the path to a
specific log format using SELECT pg_current_logfile('stderr'); for
example. This is my point of view, but if there's a majority to add the
log format into the current_logfile why not.

I have copy/paste here your other comments to limit the number of messages:

> You're writing Unix eol characters into pg_log_file.  (I think.)
> Does this matter on MS Windows?  (I'm not up on MS Windows,
> and haven't put any thought into this at all.  But didn't
> want to forget about the potential issue.)

This doesn't matter as the file is opened in text mode (see
logfile_open() in syslogger.c) and use of LF in fprintf() will be
automatically converted to CRLF on MS Windows.

> While you're at it, it wouldn't hurt to provide another
> function that tells you the format of the file returned
> by pg_current_logfile(), since pg_current_logfile()
> called without arguments could return either a stderr
> formatted file or a csvlog formatted file.

The log format can be check using something like:

postgres=# SELECT pg_current_logfile(), pg_current_logfile('csvlog');
           pg_current_logfile            |          
pg_current_logfile           
-----------------------------------------+-----------------------------------------
 pg_log/postgresql-2016-10-26_231700.log |
pg_log/postgresql-2016-10-26_231700.csv
(1 row)

postgres=# SELECT CASE WHEN (pg_current_logfile() =
pg_current_logfile('stderr')) THEN 'stderr' ELSE 'csvlog' END AS log_format;
 log_format
------------
 stderr
(1 row)

but if we want we can have an other internal function with a call like:

    SELECT pg_log_format(pg_current_logfile());

that will return stderr or csvlog but I'm not sure this is necessary.


Thanks for your review, let me know if there's any thing to adapt.

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 2e64cc4..bdae4c6 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15442,6 +15442,18 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
        <entry>configuration load time</entry>
       </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>
@@ -15660,6 +15672,29 @@ 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>
+
+   <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..b5faf52 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -61,6 +61,15 @@ Item
 </row>
 
 <row>
+ <entry><filename>current_logfile</></entry>
+ <entry>A file recording the current log file(s) used by the syslogger,
+  one file name per line.  Full path to log file(s) including the value of
+  <xref linkend="guc-log-directory"> are stored in <filename>current_logfile</>.
+  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
  <structname>pg_database</></entry>
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index fd62d66..7b09349 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,
+			 * log_destination and logging_collector may have been changed. Do
+			 * it right now to not wait for the next file rotation.
+			 */
+			logfile_writename(last_file_name, last_csv_file_name);
 		}
 
 		if (Log_RotationAge > 0 && !rotation_disabled)
@@ -513,8 +521,14 @@ SysLogger_Start(void)
 	pid_t		sysloggerPid;
 	char	   *filename;
 
-	if (!Logging_collector)
+	if (!Logging_collector) {
+		/* If logging collector is not enabled, remove current log
+		 * filename as we don't know where messages are logged and
+		 * information contained in this file are obsolete.
+		 */
+		unlink(CURRENT_LOG_FILENAME);
 		return 0;
+	}
 
 	/*
 	 * If first time through, create the pipe which will receive stderr
@@ -574,6 +588,8 @@ SysLogger_Start(void)
 
 	syslogFile = logfile_open(filename, "a", false);
 
+	logfile_writename(filename, NULL);
+
 	pfree(filename);
 
 #ifdef EXEC_BACKEND
@@ -988,8 +1004,10 @@ write_syslogger_file(const char *buffer, int count, int destination)
 	int			rc;
 	FILE	   *logfile;
 
-	if (destination == LOG_DESTINATION_CSVLOG && csvlogFile == NULL)
+	if (destination == LOG_DESTINATION_CSVLOG && csvlogFile == NULL) {
 		open_csvlogfile();
+		logfile_writename(last_file_name, last_csv_file_name);
+	}
 
 	logfile = destination == LOG_DESTINATION_CSVLOG ? csvlogFile : syslogFile;
 	rc = fwrite(buffer, 1, count, logfile);
@@ -1209,6 +1227,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 			return;
 		}
 
+		logfile_writename(filename, csvfilename);
+
 		fclose(syslogFile);
 		syslogFile = fh;
 
@@ -1220,7 +1240,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 +1272,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 			return;
 		}
 
+		logfile_writename(last_file_name, csvfilename);
+
 		fclose(csvlogFile);
 		csvlogFile = fh;
 
@@ -1365,3 +1386,60 @@ sigUsr1Handler(SIGNAL_ARGS)
 
 	errno = save_errno;
 }
+
+/*
+ * Store the name of the files 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. Filenames are first stored into a
+ * temporary file and renamed into the final destination.
+ */
+static void
+logfile_writename(char *filename, char *csvfilename)
+{
+	FILE	*fh;
+	char	tempfn[MAXPGPATH];
+	char	logpathfilename[MAXPGPATH];
+
+	snprintf(tempfn, sizeof(tempfn), "%s",
+						CURRENT_LOG_FILENAME);
+	strcat(tempfn, ".tmp");
+	snprintf(logpathfilename, sizeof(logpathfilename), "%s",
+						CURRENT_LOG_FILENAME);
+	if ((fh = logfile_open(tempfn, "w", true) ) == NULL)
+	{
+		return;
+	}
+	if (filename && (Log_destination & LOG_DESTINATION_STDERR)) {
+		if (fprintf(fh, "%s\n", filename) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					errmsg("could not write log file \"%s\": %m",
+						tempfn)));
+			fclose(fh);
+			return;
+		}
+	}
+
+	if (csvfilename && (Log_destination & LOG_DESTINATION_CSVLOG)) {
+		if (fprintf(fh, "%s\n", csvfilename) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					errmsg("could not write log file \"%s\": %m",
+						tempfn)));
+			fclose(fh);
+			return;
+		}
+	}
+	fclose(fh);
+
+	if (rename(tempfn, logpathfilename) != 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..dca711b 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,113 @@ 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    log_filename[MAXPGPATH];
+	text    *fmt;
+	char    *logfmt;
+	struct stat stat_buf;
+
+	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(log_filename, sizeof(log_filename), 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(log_filename, '\n') != NULL)
+			*strchr(log_filename, '\n') = '\0';
+
+		/*
+		 * When no log format is provided as argument or stderr format
+		 * is requested, always reports the first registered log file.
+		 * stderr log is always the first entry in CURRENT_LOG_FILENAME
+		 * see logfile_writename() * in syslogger.c . If only csvlog is
+		 * defined, log format will be rechecked later and NULL will
+		 * be returned.
+		 */
+		if ( (logfmt == NULL) || (strcmp(logfmt, "stderr") == 0) )
+			break;
+
+		/*
+		 * When the log format requested is set to csvlog and
+		 * log_destination have the csvlog format set, return
+		 * the current filename when stderr is not defined too.
+		 * If both stderr and csvlog are defined the last entry
+		 * of CURRENT_LOG_FILENAME will be reported.
+		 */
+		if ( (strcmp(logfmt, "csvlog") == 0) &&
+			(Log_destination & LOG_DESTINATION_CSVLOG) && 
+			!(Log_destination & LOG_DESTINATION_STDERR) )
+				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..15e1214 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_logfile"
+
 #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