Proposal: leave a hint when switching logging away from stderr - Mailing list pgsql-hackers

The attached patch is motivated by
http://www.postgresql.org/message-id/CAJYQwwRYt9RMBzs-sH6uCr1OTG4joXqkDF-fkoYP6pv12t0dsQ@mail.gmail.com
in which it appears that Oliver Elphick forgot to look in the configured
log_directory directory for log output, and instead examined only the file
that postmaster stderr was initially being sent to.  He's far from the
first to make such a mistake, and if a PG hacker of his standing can
forget about this, I think we've got a usability issue we ought to do
something about.

This patch arranges to emit a hint message when/if we switch away from
logging to the original postmaster stderr during startup.  There are two
cases to cover: we're still using LOG_DESTINATION_STDERR but redirecting
stderr to a syslogger process, or we stop writing to stderr altogether,
presumably in favor of going to syslog or something.

I thought about trying to leave similar breadcrumbs if the logging
parameters are changed while the postmaster is running, but it would add a
fair amount of complication to the patch, and I'm not sure there's a lot
of value in it.  On-the-fly logging parameter changes don't happen without
active DBA involvement, so it's a lot harder to credit that somebody would
not be expecting the data to start going somewhere else.

Thoughts?  In particular, anyone want to bikeshed on the message wording?

Does this rise to the level of a usability bug that ought to be
back-patched?  As I said, we've seen this type of thinko multiple
times before.

            regards, tom lane

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e6d750d..dcb04cf 100644
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*************** PostmasterMain(int argc, char *argv[])
*** 1164,1170 ****
--- 1164,1180 ----
       * Log_destination permits.  We don't do this until the postmaster is
       * fully launched, since startup failures may as well be reported to
       * stderr.
+      *
+      * If we are in fact disabling logging to stderr, first emit a log message
+      * saying so, to provide a breadcrumb trail for users who may not remember
+      * that they configured logging to go somewhere else.
       */
+     if (!(Log_destination & LOG_DESTINATION_STDERR))
+         ereport(LOG,
+                 (errmsg("ending log output to stderr"),
+                  errhint("Future log output will go to log destination \"%s\".",
+                          Log_destination_string)));
+
      whereToSendOutput = DestNone;

      /*
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index e3b6102..957843f 100644
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
*************** SysLogger_Start(void)
*** 634,639 ****
--- 634,653 ----
              /* now we redirect stderr, if not done already */
              if (!redirection_done)
              {
+ #ifdef WIN32
+                 int            fd;
+ #endif
+
+                 /*
+                  * Leave a breadcrumb trail when redirecting, since many
+                  * people have been misled by seeing some output to postmaster
+                  * stderr and thinking that that's all there is.
+                  */
+                 ereport(LOG,
+                         (errmsg("redirecting log output to logging collector process"),
+                          errhint("Look in directory \"%s\" for future log output.",
+                                  Log_directory)));
+
  #ifndef WIN32
                  fflush(stdout);
                  if (dup2(syslogPipe[1], fileno(stdout)) < 0)
*************** SysLogger_Start(void)
*** 649,656 ****
                  close(syslogPipe[1]);
                  syslogPipe[1] = -1;
  #else
-                 int            fd;
-
                  /*
                   * open the pipe in binary mode and make sure stderr is binary
                   * after it's been dup'ed into, to avoid disturbing the pipe
--- 663,668 ----
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index a812ccd..a415b90 100644
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
*************** emit_log_hook_type emit_log_hook = NULL;
*** 109,114 ****
--- 109,115 ----
  int            Log_error_verbosity = PGERROR_VERBOSE;
  char       *Log_line_prefix = NULL;        /* format for extra log line info */
  int            Log_destination = LOG_DESTINATION_STDERR;
+ char       *Log_destination_string = NULL;

  #ifdef HAVE_SYSLOG

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2b753f8..8c9e9a9 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** int            tcp_keepalives_count;
*** 444,451 ****
   * cases provide the value for SHOW to display.  The real state is elsewhere
   * and is kept in sync by assign_hooks.
   */
- static char *log_destination_string;
-
  static char *syslog_ident_str;
  static bool phony_autocommit;
  static bool session_auth_is_superuser;
--- 444,449 ----
*************** static struct config_string ConfigureNam
*** 2871,2877 ****
                           "depending on the platform."),
              GUC_LIST_INPUT
          },
!         &log_destination_string,
          "stderr",
          check_log_destination, assign_log_destination, NULL
      },
--- 2869,2875 ----
                           "depending on the platform."),
              GUC_LIST_INPUT
          },
!         &Log_destination_string,
          "stderr",
          check_log_destination, assign_log_destination, NULL
      },
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 6df0d37..59d6f5e 100644
*** a/src/include/utils/elog.h
--- b/src/include/utils/elog.h
*************** typedef enum
*** 428,433 ****
--- 428,434 ----
  extern int    Log_error_verbosity;
  extern char *Log_line_prefix;
  extern int    Log_destination;
+ extern char *Log_destination_string;

  /* Log destination bitmap */
  #define LOG_DESTINATION_STDERR     1