From 1f9158ba48122fa232db955a2ee324eec1848ba9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 29 Dec 2020 18:02:38 -0500 Subject: [PATCH] Suppress log spam from multiple reports of SIGQUIT shutdown. 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 | 16 ++++++++++++++-- src/backend/utils/error/elog.c | 10 +++++++++- src/include/utils/elog.h | 12 ++++++------ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index d35c5020ea..317d1aa573 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -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; 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 */