Re: Reopen logfile on SIGHUP - Mailing list pgsql-hackers

Hello.

At Tue, 7 Aug 2018 16:36:34 +0300, Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru> wrote in
<a9d2ad92-b4f9-42fb-77eb-f3085b964705@postgrespro.ru>
> I think the latest v4 patch addresses the concerns raised
> upthread. I'm marking the commitfest entry as RFC.

Thank you, but it is forgetting pg_ctl --help output.  I added it
and moved logrotate entry in docs just above "kill" in
app-pg-ctl.html. Also added "-s" option.

Furthermore, I did the following things.

- Since I'm not sure unlink is signal-handler safe on all
  supported platforms, I moved unlink() call out of
  checkLogrotateSignal() to SysLoggerMain, just before rotating
  log file.

- Refactored moving the main stuff to syslogger.c. 

- The diff is splitted into two files and renamed. (but version
  number continues to v5).

I'm not certain whether to allow the signal file alone cause
rotation, but this patch doesn't. Directly placed signal file is
to be removed without causing rotation.

Please find the attached.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From bee1093cc6c0361c138ebaa4920b7f62fdc4959c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 9 Aug 2018 16:09:46 +0900
Subject: [PATCH 1/2] New pg_ctl subcommand "logrotate"

We are able to force log file rotation by calling pg_rotate_logfile()
SQL function or by sending SIGUSR1 directly to syslogger. External
tools that don't want to log in as superuser needs to know the PID of
syslogger.

This patch provides a means cause log rotation by pg_ctl.
---
 src/backend/postmaster/postmaster.c |  6 ++-
 src/backend/postmaster/syslogger.c  | 36 ++++++++++++++-
 src/bin/pg_ctl/pg_ctl.c             | 89 ++++++++++++++++++++++++++++++++-----
 src/include/postmaster/syslogger.h  |  3 ++
 4 files changed, 120 insertions(+), 14 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a4b53b33cd..84928a3017 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1268,6 +1268,9 @@ PostmasterMain(int argc, char *argv[])
      */
     RemovePromoteSignalFiles();
 
+    /* Do the same for logrotate signal file */
+    RemoveLogrotateSignalFiles();
+
     /* Remove any outdated file holding the current log filenames. */
     if (unlink(LOG_METAINFO_DATAFILE) < 0 && errno != ENOENT)
         ereport(LOG,
@@ -5100,7 +5103,8 @@ sigusr1_handler(SIGNAL_ARGS)
         signal_child(PgArchPID, SIGUSR1);
     }
 
-    if (CheckPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE) &&
+    if ((CheckLogrotateSignal() ||
+         CheckPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE)) &&
         SysLoggerPID != 0)
     {
         /* Tell syslogger to rotate logfile */
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index 58b759f305..d4c41c9b7b 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -56,6 +56,9 @@
  */
 #define READ_BUF_SIZE (2 * PIPE_CHUNK_SIZE)
 
+/* Log rotation signal file path, relative to $PGDATA */
+#define LOGROTATE_SIGNAL_FILE    "logrotate"
+
 
 /*
  * GUC parameters.  Logging_collector cannot be changed after postmaster
@@ -384,11 +387,18 @@ SysLoggerMain(int argc, char *argv[])
             }
         }
 
+        /*
+         * logrotate signal file alone doesn't cause rotation. We remove the
+         * file without setting rotation_requested if any.
+         */
+        if (CheckLogrotateSignal())
+            unlink(LOGROTATE_SIGNAL_FILE);
+
         if (rotation_requested)
         {
             /*
              * Force rotation when both values are zero. It means the request
-             * was sent by pg_rotate_logfile.
+             * was sent by pg_rotate_logfile or pg_ctl logrotate.
              */
             if (!time_based_rotation && size_rotation_for == 0)
                 size_rotation_for = LOG_DESTINATION_STDERR | LOG_DESTINATION_CSVLOG;
@@ -1422,6 +1432,30 @@ update_metainfo_datafile(void)
  * --------------------------------
  */
 
+/*
+ * Check to see if a log rotation request has arrived. Should be
+ * called by postmaster after receiving SIGUSR1.
+ */
+bool
+CheckLogrotateSignal(void)
+{
+    struct stat stat_buf;
+
+    if (stat(LOGROTATE_SIGNAL_FILE, &stat_buf) == 0)
+        return true;
+
+    return false;
+}
+
+/*
+ * Remove the file signaling a log rotateion request.
+ */
+void
+RemoveLogrotateSignalFiles(void)
+{
+    unlink(LOGROTATE_SIGNAL_FILE);
+}
+
 /* SIGHUP: set flag to reload config file */
 static void
 sigHupHandler(SIGNAL_ARGS)
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index ed2396aa6c..ae8fe75c1a 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -59,6 +59,7 @@ typedef enum
     STOP_COMMAND,
     RESTART_COMMAND,
     RELOAD_COMMAND,
+    LOGROTATE_COMMAND,
     STATUS_COMMAND,
     PROMOTE_COMMAND,
     KILL_COMMAND,
@@ -100,6 +101,7 @@ static char version_file[MAXPGPATH];
 static char pid_file[MAXPGPATH];
 static char backup_file[MAXPGPATH];
 static char promote_file[MAXPGPATH];
+static char logrotate_file[MAXPGPATH];
 
 #ifdef WIN32
 static DWORD pgctl_start_type = SERVICE_AUTO_START;
@@ -125,6 +127,7 @@ static void do_restart(void);
 static void do_reload(void);
 static void do_status(void);
 static void do_promote(void);
+static void do_logrotate(void);
 static void do_kill(pgpid_t pid);
 static void print_msg(const char *msg);
 static void adjust_data_dir(void);
@@ -1171,6 +1174,62 @@ do_promote(void)
         print_msg(_("server promoting\n"));
 }
 
+/*
+ * log rotate
+ */
+
+static void
+do_logrotate(void)
+{
+    FILE       *logrotatefile;
+    pgpid_t        pid;
+
+    pid = get_pgpid(false);
+
+    if (pid == 0)                /* no pid file */
+    {
+        write_stderr(_("%s: PID file \"%s\" does not exist\n"), progname, pid_file);
+        write_stderr(_("Is server running?\n"));
+        exit(1);
+    }
+    else if (pid < 0)            /* standalone backend, not postmaster */
+    {
+        pid = -pid;
+        write_stderr(_("%s: cannot rotate log file; "
+                       "single-user server is running (PID: %ld)\n"),
+                     progname, pid);
+        exit(1);
+    }
+
+    snprintf(logrotate_file, MAXPGPATH, "%s/logrotate", pg_data);
+
+    if ((logrotatefile = fopen(logrotate_file, "w")) == NULL)
+    {
+        write_stderr(_("%s: could not create log rotation signal file \"%s\": %s\n"),
+                     progname, logrotate_file, strerror(errno));
+        exit(1);
+    }
+    if (fclose(logrotatefile))
+    {
+        write_stderr(_("%s: could not write log rotation signal file \"%s\": %s\n"),
+                     progname, logrotate_file, strerror(errno));
+        exit(1);
+    }
+
+    sig = SIGUSR1;
+    if (kill((pid_t) pid, sig) != 0)
+    {
+        write_stderr(_("%s: could not send log rotation signal (PID: %ld): %s\n"),
+                     progname, pid, strerror(errno));
+        if (unlink(logrotate_file) != 0)
+            write_stderr(_("%s: could not remove log rotation signal file \"%s\": %s\n"),
+                         progname, logrotate_file, strerror(errno));
+        exit(1);
+    }
+
+    print_msg(_("commanded to rotate log file\n"));
+}
+
 
 /*
  *    utility routines
@@ -1912,19 +1971,20 @@ do_help(void)
 {
     printf(_("%s is a utility to initialize, start, stop, or control a PostgreSQL server.\n\n"), progname);
     printf(_("Usage:\n"));
-    printf(_("  %s init[db] [-D DATADIR] [-s] [-o OPTIONS]\n"), progname);
-    printf(_("  %s start    [-D DATADIR] [-l FILENAME] [-W] [-t SECS] [-s]\n"
-             "                  [-o OPTIONS] [-p PATH] [-c]\n"), progname);
-    printf(_("  %s stop     [-D DATADIR] [-m SHUTDOWN-MODE] [-W] [-t SECS] [-s]\n"), progname);
-    printf(_("  %s restart  [-D DATADIR] [-m SHUTDOWN-MODE] [-W] [-t SECS] [-s]\n"
-             "                  [-o OPTIONS] [-c]\n"), progname);
-    printf(_("  %s reload   [-D DATADIR] [-s]\n"), progname);
-    printf(_("  %s status   [-D DATADIR]\n"), progname);
-    printf(_("  %s promote  [-D DATADIR] [-W] [-t SECS] [-s]\n"), progname);
-    printf(_("  %s kill     SIGNALNAME PID\n"), progname);
+    printf(_("  %s init[db]   [-D DATADIR] [-s] [-o OPTIONS]\n"), progname);
+    printf(_("  %s start      [-D DATADIR] [-l FILENAME] [-W] [-t SECS] [-s]\n"
+             "                    [-o OPTIONS] [-p PATH] [-c]\n"), progname);
+    printf(_("  %s stop       [-D DATADIR] [-m SHUTDOWN-MODE] [-W] [-t SECS] [-s]\n"), progname);
+    printf(_("  %s restart    [-D DATADIR] [-m SHUTDOWN-MODE] [-W] [-t SECS] [-s]\n"
+             "                    [-o OPTIONS] [-c]\n"), progname);
+    printf(_("  %s reload     [-D DATADIR] [-s]\n"), progname);
+    printf(_("  %s status     [-D DATADIR]\n"), progname);
+    printf(_("  %s promote    [-D DATADIR] [-W] [-t SECS] [-s]\n"), progname);
+    printf(_("  %s logrotate  [-D DATADIR] [-s]\n"), progname);
+    printf(_("  %s kill       SIGNALNAME PID\n"), progname);
 #ifdef WIN32
-    printf(_("  %s register [-D DATADIR] [-N SERVICENAME] [-U USERNAME] [-P PASSWORD]\n"
-             "                  [-S START-TYPE] [-e SOURCE] [-W] [-t SECS] [-s] [-o OPTIONS]\n"), progname);
+    printf(_("  %s register   [-D DATADIR] [-N SERVICENAME] [-U USERNAME] [-P PASSWORD]\n"
+             "                    [-S START-TYPE] [-e SOURCE] [-W] [-t SECS] [-s] [-o OPTIONS]\n"), progname);
     printf(_("  %s unregister [-N SERVICENAME]\n"), progname);
 #endif
 
@@ -2333,6 +2393,8 @@ main(int argc, char **argv)
                 ctl_command = RESTART_COMMAND;
             else if (strcmp(argv[optind], "reload") == 0)
                 ctl_command = RELOAD_COMMAND;
+            else if (strcmp(argv[optind], "logrotate") == 0)
+                ctl_command = LOGROTATE_COMMAND;
             else if (strcmp(argv[optind], "status") == 0)
                 ctl_command = STATUS_COMMAND;
             else if (strcmp(argv[optind], "promote") == 0)
@@ -2443,6 +2505,9 @@ main(int argc, char **argv)
         case PROMOTE_COMMAND:
             do_promote();
             break;
+        case LOGROTATE_COMMAND:
+            do_logrotate();
+            break;
         case KILL_COMMAND:
             do_kill(killproc);
             break;
diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h
index b35fadc1bd..3fcb26cdb8 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -87,6 +87,9 @@ extern void write_syslogger_file(const char *buffer, int count, int dest);
 extern void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
 #endif
 
+extern bool CheckLogrotateSignal(void);
+extern void RemoveLogrotateSignalFiles(void);
+
 /*
  * Name of files saving meta-data information about the log
  * files currently in use by the syslogger
-- 
2.16.3

From f772f45007bb6247a266fe39f0dab318b6459565 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 9 Aug 2018 16:10:35 +0900
Subject: [PATCH 2/2] Documentation for pg_ctl logrotate

---
 doc/src/sgml/maintenance.sgml    | 34 ++++++++++++++++++++++++++++++++--
 doc/src/sgml/ref/pg_ctl-ref.sgml | 15 ++++++++++++++-
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml
index 4a68ec3b40..fd538e1cce 100644
--- a/doc/src/sgml/maintenance.sgml
+++ b/doc/src/sgml/maintenance.sgml
@@ -932,8 +932,8 @@ analyze threshold = analyze base threshold + analyze scale factor * number of tu
    program if you have one that you are already using with other
    server software. For example, the <application>rotatelogs</application>
    tool included in the <productname>Apache</productname> distribution
-   can be used with <productname>PostgreSQL</productname>.  To do this,
-   just pipe the server's
+   can be used with <productname>PostgreSQL</productname>. One way to 
+   do this is to pipe the server's
    <systemitem>stderr</systemitem> output to the desired program.
    If you start the server with
    <command>pg_ctl</command>, then <systemitem>stderr</systemitem>
@@ -945,6 +945,36 @@ pg_ctl start | rotatelogs /var/log/pgsql_log 86400
 </programlisting>
   </para>
 
+  <para>
+   You can combine these approaches by setting up <application>logrotate</application>
+   to collect log files produced by <productname>PostgreSQL</productname> built-in logging
+   collector. In this case, the logging collector defines the names and
+   location of the log files, while <application>logrotate</application>
+   periodically archives these files. When initiating log rotation,
+   <application>logrotate</application> must ensure that the application
+   sends further output to the new file. This is commonly done with a
+   <literal>postrotate</literal> script that sends a <literal>SIGHUP</literal> signal to
+   the application, which then reopens the log file.
+   In <productname>PostgreSQL</productname>, you can run <application>pg_ctl</application>
+   with the <literal>logrotate</literal> option instead. When the server receives
+   this command, the server either switches to a new log file or reopens the
+   existing file, depending on the logging configuration
+   (see <xref linkend="runtime-config-logging-where"/>).
+  </para>
+
+  <note>
+   <para>
+    When using static log file names, the server might fail to reopen the log
+    file if the max open file limit is reached or a file table overflow occurs.
+    In this case, log messages are sent to the old log file until a
+    successful log rotation. If <application>logrotate</application> is
+    configured to compress the log file and delete it, the server may lose
+    the messages logged in this timeframe. To avoid this issue, you can
+    configure the logging collector to dynamically assign log file names
+    and use a <literal>prerotate</literal> script to ignore open log files.
+    </para>
+  </note>
+
   <para>
    Another production-grade approach to managing log output is to
    send it to <application>syslog</application> and let
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 01a9ba4542..fbbcc7b8e7 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -97,12 +97,19 @@ PostgreSQL documentation
    <arg choice="opt"><option>-s</option></arg>
   </cmdsynopsis>
 
+  <cmdsynopsis>
+   <command>pg_ctl</command>
+   <arg choice="plain"><option>logrotate</option></arg>
+   <arg choice="opt"><option>-D</option> <replaceable>datadir</replaceable></arg>
+   <arg choice="opt"><option>-s</option></arg>
+  </cmdsynopsis>
+
   <cmdsynopsis>
    <command>pg_ctl</command>
    <arg choice="plain"><option>kill</option></arg>
    <arg choice="plain"><replaceable>signal_name</replaceable></arg>
    <arg choice="plain"><replaceable>process_id</replaceable></arg>
-  </cmdsynopsis>
+  </cmdsynopsis>  
 
   <para>On Microsoft Windows, also:</para>
 
@@ -226,6 +233,12 @@ PostgreSQL documentation
    and begin read-write operations.
   </para>
 
+  <para>
+   <option>logrotate</option> mode rotates the server log file.
+   For details on how to use this mode with external log rotation tools, see
+   <xref linkend="logfile-maintenance"/>.
+  </para>
+
   <para>
    <option>kill</option> mode sends a signal to a specified process.
    This is primarily valuable on <productname>Microsoft Windows</productname>
-- 
2.16.3