From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Better client reporting for "immediate stop" shutdowns
In-reply-to: <20201226191627.nowyox3chvk4mjhk@alap3.anarazel.de>
References: <559291.1608587013@sss.pgh.pa.us>
 <20201225230331.hru3u6obyy6j53tk@alap3.anarazel.de>
 <1293510.1609007835@sss.pgh.pa.us>
 <20201226191627.nowyox3chvk4mjhk@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Sat, 26 Dec 2020 11:16:27 -0800"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1730605.1609179858.0@sss.pgh.pa.us>
Date: Mon, 28 Dec 2020 13:25:14 -0500
Message-ID: <1730644.1609179914@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1730605.1609179858.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="suppress-log-spam-for-SIGQUIT-1.patch";
	charset="us-ascii"
Content-ID: <1730605.1609179858.2@sss.pgh.pa.us>
Content-Description: suppress-log-spam-for-SIGQUIT-1.patch
Content-Transfer-Encoding: quoted-printable

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 p=
rocess"),
 					 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 =3D=3D LOG || log_min_level <=3D ERROR)
 			return true;
 	}
+	else if (elevel =3D=3D WARNING_CLIENT_ONLY)
+	{
+		/* never sent to log, regardless of log_min_level */
+		return false;
+	}
 	else if (log_min_level =3D=3D LOG)
 	{
 		/* elevel !=3D LOG */
@@ -453,7 +458,7 @@ errstart(int elevel, const char *domain)
 	/* Select default errcode based on elevel */
 	if (elevel >=3D ERROR)
 		edata->sqlerrcode =3D ERRCODE_INTERNAL_ERROR;
-	else if (elevel =3D=3D WARNING)
+	else if (elevel >=3D WARNING)
 		edata->sqlerrcode =3D ERRCODE_WARNING;
 	else
 		edata->sqlerrcode =3D ERRCODE_SUCCESSFUL_COMPLETION;
@@ -2152,6 +2157,7 @@ write_eventlog(int level, const char *line, int len)
 			eventlevel =3D EVENTLOG_INFORMATION_TYPE;
 			break;
 		case WARNING:
+		case WARNING_CLIENT_ONLY:
 			eventlevel =3D 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 =3D LOG_NOTICE;
 				break;
 			case ERROR:
@@ -3484,6 +3491,7 @@ error_severity(int elevel)
 			prefix =3D gettext_noop("NOTICE");
 			break;
 		case WARNING:
+		case WARNING_CLIENT_ONLY:
 			prefix =3D 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 */

------- =_aaaaaaaaaa0--