Re: Better client reporting for "immediate stop" shutdowns - Mailing list pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> On 2020-12-26 13:37:15 -0500, Tom Lane wrote:
>>> I'd like to not log all these repeated messages into the server
>>> log. It's quite annoying to have to digg through thousands of lines of
>>> repeated "terminating connection..."

>> Hm.  That's an orthogonal issue, but certainly worth considering.
>> There are a couple of levels we could consider:
>> 1. Just make the logged messages less verbose (they certainly don't
>> need the DETAIL and HINT lines).
>> 2. Suppress the log entries altogether.

> My vote would be #2, with the same reasoning as yours.

The most straightforward way to do that is to introduce a new error
level.  Having to renumber existing levels is a bit of a pain, but
I'm not aware of anything that should break in source-code terms.
We make similar ABI breaks in every major release.

            regards, tom lane

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index d35c5020ea..e9504cdab9 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2791,6 +2791,10 @@ quickdie(SIGNAL_ARGS)
      *
      * Ideally these should be ereport(FATAL), but then we'd not get control
      * back to force the correct type of process exit.
+     *
+     * When responding to a postmaster-issued signal, send the message only to
+     * the client; sending to the server log just creates log spam, plus it's
+     * more code that we need to hope will work in a signal handler.
      */
     switch (GetQuitSignalReason())
     {
@@ -2802,7 +2806,7 @@ quickdie(SIGNAL_ARGS)
             break;
         case PMQUIT_FOR_CRASH:
             /* A crash-and-restart cycle is in progress */
-            ereport(WARNING,
+            ereport(WARNING_CLIENT_ONLY,
                     (errcode(ERRCODE_CRASH_SHUTDOWN),
                      errmsg("terminating connection because of crash of another server process"),
                      errdetail("The postmaster has commanded this server process to roll back"
@@ -2814,7 +2818,7 @@ quickdie(SIGNAL_ARGS)
             break;
         case PMQUIT_FOR_STOP:
             /* Immediate-mode stop */
-            ereport(WARNING,
+            ereport(WARNING_CLIENT_ONLY,
                     (errcode(ERRCODE_ADMIN_SHUTDOWN),
                      errmsg("terminating connection due to immediate shutdown command")));
             break;
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 3558e660c7..9a69038b80 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -202,6 +202,11 @@ is_log_level_output(int elevel, int log_min_level)
         if (log_min_level == LOG || log_min_level <= ERROR)
             return true;
     }
+    else if (elevel == WARNING_CLIENT_ONLY)
+    {
+        /* never sent to log, regardless of log_min_level */
+        return false;
+    }
     else if (log_min_level == LOG)
     {
         /* elevel != LOG */
@@ -453,7 +458,7 @@ errstart(int elevel, const char *domain)
     /* Select default errcode based on elevel */
     if (elevel >= ERROR)
         edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
-    else if (elevel == WARNING)
+    else if (elevel >= WARNING)
         edata->sqlerrcode = ERRCODE_WARNING;
     else
         edata->sqlerrcode = ERRCODE_SUCCESSFUL_COMPLETION;
@@ -2152,6 +2157,7 @@ write_eventlog(int level, const char *line, int len)
             eventlevel = EVENTLOG_INFORMATION_TYPE;
             break;
         case WARNING:
+        case WARNING_CLIENT_ONLY:
             eventlevel = EVENTLOG_WARNING_TYPE;
             break;
         case ERROR:
@@ -3109,6 +3115,7 @@ send_message_to_server_log(ErrorData *edata)
                 break;
             case NOTICE:
             case WARNING:
+            case WARNING_CLIENT_ONLY:
                 syslog_level = LOG_NOTICE;
                 break;
             case ERROR:
@@ -3484,6 +3491,7 @@ error_severity(int elevel)
             prefix = gettext_noop("NOTICE");
             break;
         case WARNING:
+        case WARNING_CLIENT_ONLY:
             prefix = gettext_noop("WARNING");
             break;
         case ERROR:
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index e8f04a1691..d2bdfa0be3 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -40,19 +40,19 @@
 #define WARNING        19            /* Warnings.  NOTICE is for expected messages
                                  * like implicit sequence creation by SERIAL.
                                  * WARNING is for unexpected messages. */
-#define ERROR        20            /* user error - abort transaction; return to
+#define WARNING_CLIENT_ONLY    20    /* Warnings to be sent to client as usual, but
+                                 * never to the server log. */
+#define ERROR        21            /* user error - abort transaction; return to
                                  * known state */
 /* Save ERROR value in PGERROR so it can be restored when Win32 includes
  * modify it.  We have to use a constant rather than ERROR because macros
  * are expanded only when referenced outside macros.
  */
 #ifdef WIN32
-#define PGERROR        20
+#define PGERROR        21
 #endif
-#define FATAL        21            /* fatal error - abort process */
-#define PANIC        22            /* take down the other backends with me */
-
- /* #define DEBUG DEBUG1 */    /* Backward compatibility with pre-7.3 */
+#define FATAL        22            /* fatal error - abort process */
+#define PANIC        23            /* take down the other backends with me */


 /* macros for representing SQLSTATE strings compactly */