On Sun, 15 Jan 2017 07:20:40 -0600
"Karl O. Pinc" <k...@meme.com> wrote:

> On Sun, 15 Jan 2017 14:54:44 +0900
> Michael Paquier <michael.paqu...@gmail.com> wrote:
> 
> > I think that's all I have for this patch.  

> I'd like to submit with it an addendum patch that
> makes symbols out of some constants.  I'll see if I can
> get that done soon.

Attached is a version 23 of the patch.  The only change
is follow-through and cleanup of code posted in-line in emails.

  src/backend/utils/adt/misc.c | 13 ++++++-------
  1 file changed, 6 insertions(+), 7 deletions(-)

This includes making comments into full sentences.


I do have a question here regards code formatting.
The patch now contains:

    if (log_filepath == NULL)
    {
        /* Bad data. Avoid segfaults etc. and return NULL to caller. */
        break;
    }

I'm not sure how this fits in with PG coding style,
whether the {} should be removed or what.  I've looked
around and can't find an example of an if with a single
line then block and a comment.  Maybe this means that
I shouldn't be doing this, but if I put the comment above
the if then it will "run into" the comment block which
immediately precedes the above code which describes
a larger body of code.  So perhaps someone should look
at this and tell me how to improve it.


Attached also are 2 patches which abstract some hardcoded
constants into symbols.  Whether to apply them is a matter
of style and left to the maintainers, which is why these
are separate patches.  

The first patch changes only code on the master
branch, the 2nd patch changes the new code.


I have not looked further at the patch or checked
to see that all changes previously recommended have been
made.  Michael, if you're confident that everything is good
go ahead and move the patchs forward to the maintainers.  Otherwise
please let me know and I'll see about finding time
for further review.

Regards,

Karl <k...@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 07afa3c..c44984d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4211,6 +4211,17 @@ SELECT * FROM parent WHERE key = 2400;
      <primary>server log</primary>
     </indexterm>
 
+    <para>
+     When logs are written to the file system, the <xref
+     linkend="storage-pgdata-current-logfiles"> file includes the type,
+     the location and the name of the logs currently in use. This provides a
+     convenient way to find the logs currently in used by the instance.
+<programlisting>
+$ cat $PGDATA/current_logfiles
+stderr pg_log/postgresql-2017-01-13_085812.log
+</programlisting>
+    </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 10e3186..693669b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -15441,6 +15441,13 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
       </row>
 
       <row>
+       <entry><literal><function>pg_current_logfile(<optional><type>text</></optional>)</function></literal></entry>
+       <entry><type>text</type></entry>
+       <entry>Primary log file name, or log in the requested format,
+       currently 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>
@@ -15658,6 +15665,39 @@ 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 log files 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">.
+
+    <function>pg_current_logfiles</function> reflects the content of the
+    <xref linkend="storage-pgdata-current-logfiles"> file.  All caveats
+    regards <filename>current_logfiles</filename> content are applicable
+    to <function>pg_current_logfiles</function>' return value.
+   </para>
+
    <indexterm>
     <primary>pg_my_temp_schema</primary>
    </indexterm>
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 5c52824..3ce2a7e 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -60,6 +60,38 @@ 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>
+  <para>
+  A file recording the log file(s) currently written to by the syslogger
+  and the file's log formats, <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
+  <filename>current_logfiles</filename>.
+  </para>
+
+  <para>
+  The <filename>current_logfiles</filename> file
+  is present only when <xref linkend="guc-logging-collector"> is
+  activated and when at least one of <systemitem>stderr</> or
+  <systemitem>csvlog</> value is present in
+  <xref linkend="guc-log-destination">.
+  </para>
+ </entry>
+</row>
+
 <row>
  <entry><filename>global</></entry>
  <entry>Subdirectory containing cluster-wide tables, such as
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 31aade1..1d7f68b 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1055,6 +1055,7 @@ REVOKE EXECUTE ON FUNCTION pg_xlog_replay_pause() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_xlog_replay_resume() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_rotate_logfile() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_reload_conf() FROM public;
+REVOKE EXECUTE ON FUNCTION pg_current_logfile() FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_stat_reset() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_stat_reset_shared(text) FROM public;
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index 13a0301..a92dccb 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -146,7 +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 update_metainfo_datafile(void);
 
 /*
  * Main entry point for syslogger process
@@ -348,6 +348,12 @@ 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.
+			 */
+			update_metainfo_datafile();
 		}
 
 		if (Log_RotationAge > 0 && !rotation_disabled)
@@ -511,10 +517,16 @@ int
 SysLogger_Start(void)
 {
 	pid_t		sysloggerPid;
-	char	   *filename;
 
+	/*
+	 * Logging collector is not enabled. We don't know where messages are
+	 * logged.  Remove outdated file holding the current log filenames.
+	 */
 	if (!Logging_collector)
+	{
+		unlink(LOG_METAINFO_DATAFILE);
 		return 0;
+	}
 
 	/*
 	 * If first time through, create the pipe which will receive stderr
@@ -570,11 +582,15 @@ SysLogger_Start(void)
 	 * a time-based rotation.
 	 */
 	first_syslogger_file_time = time(NULL);
-	filename = logfile_getname(first_syslogger_file_time, NULL);
 
-	syslogFile = logfile_open(filename, "a", false);
+	if (last_file_name != NULL)
+		pfree(last_file_name);
+
+	last_file_name = logfile_getname(first_syslogger_file_time, NULL);
+
+	syslogFile = logfile_open(last_file_name, "a", false);
 
-	pfree(filename);
+	update_metainfo_datafile();
 
 #ifdef EXEC_BACKEND
 	switch ((sysloggerPid = syslogger_forkexec()))
@@ -1098,6 +1114,8 @@ open_csvlogfile(void)
 		pfree(last_csv_file_name);
 
 	last_csv_file_name = filename;
+
+	update_metainfo_datafile();
 }
 
 /*
@@ -1268,6 +1286,8 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 	if (csvfilename)
 		pfree(csvfilename);
 
+	update_metainfo_datafile();
+
 	set_next_rotation_time();
 }
 
@@ -1365,3 +1385,61 @@ 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
+update_metainfo_datafile()
+{
+	FILE    *fh;
+
+	if (!(Log_destination & LOG_DESTINATION_STDERR)
+		&& !(Log_destination & LOG_DESTINATION_CSVLOG))
+	{
+		unlink(LOG_METAINFO_DATAFILE);
+		return;
+	}
+
+	if ((fh = logfile_open(LOG_METAINFO_DATAFILE_TMP, "w", true)) == NULL)
+		return;
+
+	if (last_file_name && (Log_destination & LOG_DESTINATION_STDERR))
+	{
+		if (fprintf(fh, "stderr %s\n", last_file_name) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					errmsg("could not write stderr log file path \"%s\": %m",
+							LOG_METAINFO_DATAFILE_TMP)));
+			fclose(fh);
+			return;
+		}
+	}
+
+	if (last_csv_file_name && (Log_destination & LOG_DESTINATION_CSVLOG))
+	{
+		if (fprintf(fh, "csvlog %s\n", last_csv_file_name) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					errmsg("could not write csvlog log file path \"%s\": %m",
+							LOG_METAINFO_DATAFILE_TMP)));
+			fclose(fh);
+			return;
+		}
+	}
+	fclose(fh);
+
+	if (rename(LOG_METAINFO_DATAFILE_TMP, LOG_METAINFO_DATAFILE) != 0)
+	{
+		ereport(LOG,
+				(errcode_for_file_access(),
+				errmsg("could not rename file \"%s\": %m",
+						LOG_METAINFO_DATAFILE_TMP)));
+		return;
+	}
+}
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 09ecc15..4152d30 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -37,6 +37,7 @@
 #include "utils/elog.h"
 #include "utils/ps_status.h"
 #include "utils/timestamp.h"
+#include "postmaster/syslogger.h"
 
 
 typedef struct
@@ -148,6 +149,9 @@ static const char *excludeFiles[] =
 	/* Skip auto conf temporary file. */
 	PG_AUTOCONF_FILENAME ".tmp",
 
+	/* Skip current log file temporary file */
+	LOG_METAINFO_DATAFILE_TMP,
+
 	/*
 	 * If there's a backup_label or tablespace_map file, it belongs to a
 	 * backup started by the user with pg_start_backup().  It is *not* correct
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 66d09bc..68dd66f 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -892,3 +892,98 @@ 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];
+	char    *logfmt;
+	char    *log_filepath;
+	char    *log_format = lbuffer;
+
+	/* The log format parameter is optional */
+	if (PG_NARGS() == 0 || PG_ARGISNULL(0))
+		logfmt = NULL;
+	else
+	{
+		logfmt = text_to_cstring(PG_GETARG_TEXT_PP(0));
+		if (strcmp(logfmt, "stderr") != 0 && strcmp(logfmt, "csvlog") != 0)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("log format \"%s\" is not supported", logfmt),
+					 errhint("The supported log formats are \"stderr\""
+									" and \"csvlog\".")));
+			PG_RETURN_NULL();
+		}
+	}
+
+	fd = AllocateFile(LOG_METAINFO_DATAFILE, "r");
+	if (fd == NULL)
+	{
+		if (errno != ENOENT)
+			ereport(ERROR,
+					(errcode_for_file_access(),
+					 errmsg("could not read file \"%s\": %m",
+						LOG_METAINFO_DATAFILE)));
+		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",
+							LOG_METAINFO_DATAFILE)));
+		}
+
+		/*
+		 * Extract log format and log file path from the line;
+		 * lbuffer == log_format, they share storage.
+		 */
+		log_filepath = strchr(lbuffer, ' ');
+		if (log_filepath == NULL)
+		{
+			/* Bad data. Avoid segfaults etc. and return NULL to caller. */
+			break;
+		}
+		*log_filepath = '\0';
+		log_filepath++;
+		log_filepath[strcspn(log_filepath, "\n")] = '\0';
+
+		if (logfmt == NULL || strcmp(logfmt, log_format) == 0)
+		{
+			FreeFile(fd);
+			PG_RETURN_TEXT_P(cstring_to_text(log_filepath));
+		}
+	}
+
+	/* Close the current log filename file. */
+	FreeFile(fd);
+
+	PG_RETURN_NULL();
+}
+
+/*
+ * Report current log file used by log collector (1 argument version)
+ *
+ * note: this wrapper is necessary to pass the sanity check in opr_sanity,
+ * which checks that all built-in functions that share the implementing C
+ * function take the same number of arguments
+ */
+Datum
+pg_current_logfile_1arg(PG_FUNCTION_ARGS)
+{
+	return pg_current_logfile(fcinfo);
+}
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 4c6670c..5194c74 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -56,7 +56,7 @@ close CONF;
 $node->restart;
 
 # Write some files to test that they are not copied.
-foreach my $filename (qw(backup_label tablespace_map postgresql.auto.conf.tmp))
+foreach my $filename (qw(backup_label tablespace_map postgresql.auto.conf.tmp current_logfiles.tmp))
 {
 	open FILE, ">>$pgdata/$filename";
 	print FILE "DONOTCOPY";
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 37e022d..eb56e8d 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3183,6 +3183,10 @@ 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 0 0 25 "" _null_ _null_ _null_ _null_ _null_ pg_current_logfile _null_ _null_ _null_ ));
+DESCR("current logging collector file location");
+DATA(insert OID = 3801 ( 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_1arg _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/postmaster/syslogger.h b/src/include/postmaster/syslogger.h
index c187a5f..ebe3247 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -87,4 +87,11 @@ extern void write_syslogger_file(const char *buffer, int count, int dest);
 extern void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
 #endif
 
+/*
+ * Name of files saving meta-data information about the log
+ * files currently in use by the system logging process
+ */
+#define LOG_METAINFO_DATAFILE  "current_logfiles"
+#define LOG_METAINFO_DATAFILE_TMP  LOG_METAINFO_DATAFILE ".tmp"
+
 #endif   /* _SYSLOGGER_H */
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index e1bb344..e2532e6 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -522,6 +522,8 @@ 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);
+extern Datum pg_current_logfile_1arg(PG_FUNCTION_ARGS);
 
 /* oid.c */
 extern Datum oidin(PG_FUNCTION_ARGS);

Attachment: patch_pg_current_logfile-v23.diff.abstract_guc_part1
Description: Binary data

Attachment: patch_pg_current_logfile-v23.diff.abstract_guc_part2
Description: Binary data

-- 
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