Suppress log spam from multiple reports of SIGQUIT shutdown.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 29 Dec 2020 23:02:38 +0000 (18:02 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 29 Dec 2020 23:02:38 +0000 (18:02 -0500)
When the postmaster sends SIGQUIT to its children, there's no real
need for all the children to log that fact; the postmaster already
made a log entry about it, so adding perhaps dozens or hundreds of
child-process log entries adds nothing of value.  So, let's introduce
a new ereport level to specify "WARNING, but never send to log" and
use that for these messages.

Such a change wouldn't have been desirable before commit 7e784d1dc,
because if someone manually SIGQUIT's a backend, we *do* want to log
that.  But now we can tell the difference between a signal that was
issued by the postmaster and one that was not with reasonable
certainty.

While we're here, also clear error_context_stack before ereport'ing,
to prevent error callbacks from being invoked in the signal-handler
context.  This should reduce the odds of getting hung up while trying
to notify the client.

Per a suggestion from Andres Freund.

Discussion: https://postgr.es/m/20201225230331.hru3u6obyy6j53tk@alap3.anarazel.de

src/backend/tcop/postgres.c
src/backend/utils/error/elog.c
src/include/utils/elog.h

index d35c5020ea634d5bbb301f14b59a23b03d269434..317d1aa57309fe24ea16d0de9f555efbb1b8c1d8 100644 (file)
@@ -2789,6 +2789,18 @@ quickdie(SIGNAL_ARGS)
         * wrong, so there's not much to lose.  Assuming the postmaster is still
         * running, it will SIGKILL us soon if we get stuck for some reason.
         *
+        * One thing we can do to make this a tad safer is to clear the error
+        * context stack, so that context callbacks are not called.  That's a lot
+        * less code that could be reached here, and the context info is unlikely
+        * to be very relevant to a SIGQUIT report anyway.
+        */
+       error_context_stack = NULL;
+
+       /*
+        * When responding to a postmaster-issued signal, we 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.
+        *
         * Ideally these should be ereport(FATAL), but then we'd not get control
         * back to force the correct type of process exit.
         */
@@ -2802,7 +2814,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 +2826,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;
index 3558e660c73b2fbcc3a4f17f7245f77cc71f5938..9a69038b80c7cb8d3dfbbb782d593447cda5f469 100644 (file)
@@ -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:
index e8f04a1691649114c89549be9bff8e687be98901..d2bdfa0be3a9740df1ff091c8583476d154f5ea7 100644 (file)
 #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 */