Fix syslogger so that log_truncate_on_rotation works in the first rotation.

In the original coding of the log rotation stuff, we did not bother to make
the truncation logic work for the very first rotation after postmaster
start (or after a syslogger crash and restart).  It just always appended
in that case.  It did not seem terribly important at the time, but we've
recently had two separate complaints from people who expected it to work
unsurprisingly.  (Both users tend to restart the postmaster about as often
as a log rotation is configured to happen, which is maybe not typical use,
but still...)  Since the initial log file is opened in the postmaster,
fixing this requires passing down some more state to the syslogger child
process.

It's always been like this, so back-patch to all supported branches.

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ead51ee164e435f71f1ecfe0684d9a463e9cdd49..5ae92d714005c78757127b214fa5cbb7dc129299 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -436,6 +436,7 @@ typedef struct
        pid_t           PostmasterPid;
        TimestampTz PgStartTime;
        TimestampTz PgReloadTime;
+       pg_time_t       first_syslogger_file_time;
        bool            redirection_done;
 #ifdef WIN32
        HANDLE          PostmasterHandle;
@@ -4624,7 +4625,7 @@ MaxLivePostmasterChildren(void)
 
 /*
  * The following need to be available to the save/restore_backend_variables
- * functions
+ * functions.  They are marked NON_EXEC_STATIC in their home modules.
  */
 extern slock_t *ShmemLock;
 extern LWLock *LWLockArray;
@@ -4633,6 +4634,7 @@ extern PROC_HDR *ProcGlobal;
 extern PGPROC *AuxiliaryProcs;
 extern PMSignalData *PMSignalState;
 extern pgsocket pgStatSock;
+extern pg_time_t first_syslogger_file_time;
 
 #ifndef WIN32
 #define write_inheritable_socket(dest, src, childpid) ((*(dest) = (src)), true)
@@ -4684,6 +4686,7 @@ save_backend_variables(BackendParameters * param, Port *port,
        param->PostmasterPid = PostmasterPid;
        param->PgStartTime = PgStartTime;
        param->PgReloadTime = PgReloadTime;
+       param->first_syslogger_file_time = first_syslogger_file_time;
 
        param->redirection_done = redirection_done;
 
@@ -4902,6 +4905,7 @@ restore_backend_variables(BackendParameters * param, Port *port)
        PostmasterPid = param->PostmasterPid;
        PgStartTime = param->PgStartTime;
        PgReloadTime = param->PgReloadTime;
+       first_syslogger_file_time = param->first_syslogger_file_time;
 
        redirection_done = param->redirection_done;
 

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index 0c6ae1947695ccc9a0e79d26b01841e24c4c8636..3a1ff4537f4206d0a4160963a659b4b1c96c52cc 100644 (file)
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -2,7 +2,7 @@
  *
  * syslogger.c
  *
- * The system logger (syslogger) is new in Postgres 8.0. It catches all
+ * The system logger (syslogger) appeared in Postgres 8.0. It catches all
  * stderr output from the postmaster, backends, and other subprocesses
  * by redirecting to a pipe, and writes it to a set of logfiles.
  * It's possible to have size and age limits for the logfile configured
@@ -90,6 +90,7 @@ static bool pipe_eof_seen = false;
 static bool rotation_disabled = false;
 static FILE *syslogFile = NULL;
 static FILE *csvlogFile = NULL;
+NON_EXEC_STATIC pg_time_t first_syslogger_file_time = 0;
 static char *last_file_name = NULL;
 static char *last_csv_file_name = NULL;
 
@@ -281,6 +282,13 @@ SysLoggerMain(int argc, char *argv[])
                elog(FATAL, "could not create syslogger data transfer thread: %m");
 #endif   /* WIN32 */
 
+       /*
+        * Remember active logfile's name.  We recompute this from the reference
+        * time because passing down just the pg_time_t is a lot cheaper than
+        * passing a whole file path in the EXEC_BACKEND case.
+        */
+       last_file_name = logfile_getname(first_syslogger_file_time, NULL);
+
        /* remember active logfile parameters */
        currentLogDir = pstrdup(Log_directory);
        currentLogFilename = pstrdup(Log_filename);
@@ -532,9 +540,18 @@ SysLogger_Start(void)
 
        /*
         * The initial logfile is created right in the postmaster, to verify that
-        * the Log_directory is writable.
+        * the Log_directory is writable.  We save the reference time so that
+        * the syslogger child process can recompute this file name.
+        *
+        * It might look a bit strange to re-do this during a syslogger restart,
+        * but we must do so since the postmaster closed syslogFile after the
+        * previous fork (and remembering that old file wouldn't be right anyway).
+        * Note we always append here, we won't overwrite any existing file.  This
+        * is consistent with the normal rules, because by definition this is not
+        * a time-based rotation.
         */
-       filename = logfile_getname(time(NULL), NULL);
+       first_syslogger_file_time = time(NULL);
+       filename = logfile_getname(first_syslogger_file_time, NULL);
 
        syslogFile = fopen(filename, "a");
 
@@ -1022,8 +1039,12 @@ pipeThread(void *arg)
 #endif   /* WIN32 */
 
 /*
- * open the csv log file - we do this opportunistically, because
+ * Open the csv log file - we do this opportunistically, because
  * we don't know if CSV logging will be wanted.
+ *
+ * This is only used the first time we open the csv log in a given syslogger
+ * process, not during rotations.  As with opening the main log file, we
+ * always append in this situation.
  */
 static void
 open_csvlogfile(void)
@@ -1049,8 +1070,10 @@ open_csvlogfile(void)
 
        csvlogFile = fh;
 
-       pfree(filename);
+       if (last_csv_file_name != NULL)         /* probably shouldn't happen */
+               pfree(last_csv_file_name);
 
+       last_csv_file_name = filename;
 }
 
 /*
@@ -1085,14 +1108,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
         * elapsed time and not something else, and (c) the computed file name is
         * different from what we were previously logging into.
         *
-        * Note: during the first rotation after forking off from the postmaster,
-        * last_file_name will be NULL.  (We don't bother to set it in the
-        * postmaster because it ain't gonna work in the EXEC_BACKEND case.) So we
-        * will always append in that situation, even though truncating would
-        * usually be safe.
-        *
-        * For consistency, we treat CSV logs the same even though they aren't
-        * opened in the postmaster.
+        * Note: last_file_name should never be NULL here, but if it is, append.
         */
        if (time_based_rotation || (size_rotation_for & LOG_DESTINATION_STDERR))
        {