Hi Tom,
On 3/29/16 12:58 PM, Tom Lane wrote:
Oh, I agree that there's a compelling use-case for LOG |
ERR_HIDE_FROM_CLIENT. I'm less certain that there's a use-case
for supporting such a flag across all elevels that is strong enough
to justify all the work we'd have to do to make it happen. Basically,
my point is that LOG_ONLY achieves 95% of the benefit for probably
0.01% of the work.
Attached is a patch that re-purposes COMMERROR as LOG_SERVER_ONLY. I
went ahead and replaced all instances of COMMERROR with LOG_SERVER_ONLY.
I left the COMMERROR #define but it is no longer used by any server,
client, or included contrib code (I also noted that it was DEPRECATED in
the comment). I'll leave it up to the committer to remove if deemed
appropriate.
I realize there's no agreement on how this should work ideally, but this
patch answers the current need without introducing anything new and
shouldn't cause regressions. It does address confusion that would arise
from using COMMERROR in ereports that clearly are not.
Thanks,
--
-David
[email protected]
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 2751183..db6da9f 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -635,7 +635,7 @@ recv_password_packet(Port *port)
* log.
*/
if (mtype != EOF)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("expected password response, got
message type %d",
mtype)));
@@ -663,7 +663,7 @@ recv_password_packet(Port *port)
* StringInfo is guaranteed to have an appended '\0'.
*/
if (strlen(buf.data) + 1 != buf.len)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid password packet size")));
@@ -853,7 +853,7 @@ pg_GSS_recvauth(Port *port)
{
/* Only log error if client didn't disconnect. */
if (mtype != EOF)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("expected GSS response,
got message type %d",
mtype)));
@@ -1092,7 +1092,7 @@ pg_SSPI_recvauth(Port *port)
{
/* Only log error if client didn't disconnect. */
if (mtype != EOF)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("expected SSPI
response, got message type %d",
mtype)));
diff --git a/src/backend/libpq/be-secure-openssl.c
b/src/backend/libpq/be-secure-openssl.c
index 6009663..b8f7a07 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -359,7 +359,7 @@ be_tls_open_server(Port *port)
if (!(port->ssl = SSL_new(SSL_context)))
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not initialize SSL connection:
%s",
SSLerrmessage())));
@@ -367,7 +367,7 @@ be_tls_open_server(Port *port)
}
if (!my_SSL_set_fd(port, port->sock))
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not set SSL socket: %s",
SSLerrmessage())));
@@ -401,27 +401,27 @@ aloop:
goto aloop;
case SSL_ERROR_SYSCALL:
if (r < 0)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode_for_socket_access(),
errmsg("could not
accept SSL connection: %m")));
else
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not accept SSL
connection: EOF detected")));
break;
case SSL_ERROR_SSL:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not accept SSL
connection: %s",
SSLerrmessage())));
break;
case SSL_ERROR_ZERO_RETURN:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not accept SSL connection: EOF
detected")));
break;
default:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unrecognized SSL error
code: %d",
err)));
@@ -465,7 +465,7 @@ aloop:
*/
if (len != strlen(peer_cn))
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("SSL certificate's
common name contains embedded null")));
pfree(peer_cn);
@@ -550,7 +550,7 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
}
break;
case SSL_ERROR_SSL:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("SSL error: %s",
SSLerrmessage())));
/* fall through */
@@ -559,7 +559,7 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
n = -1;
break;
default:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unrecognized SSL error code:
%d",
err)));
@@ -607,7 +607,7 @@ be_tls_write(Port *port, void *ptr, size_t len, int
*waitfor)
}
break;
case SSL_ERROR_SSL:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("SSL error: %s",
SSLerrmessage())));
/* fall through */
@@ -616,7 +616,7 @@ be_tls_write(Port *port, void *ptr, size_t len, int
*waitfor)
n = -1;
break;
default:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unrecognized SSL error code:
%d",
err)));
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 8d6eb0b..5a94af1 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -192,13 +192,13 @@ pq_init(void)
* needed. That allows us to provide safely interruptible reads and
* writes.
*
- * Use COMMERROR on failure, because ERROR would try to send the error
to
+ * Use LOG_SERVER_ONLY on failure, because ERROR would try to send the
error to
* the client, which might require changing the mode again, leading to
* infinite recursion.
*/
#ifndef WIN32
if (!pg_set_noblock(MyProcPort->sock))
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errmsg("could not set socket to nonblocking
mode: %m")));
#endif
@@ -931,7 +931,7 @@ pq_recvbuf(void)
* cause recursion to here, leading to stack overflow
and core
* dump! This message must go *only* to the postmaster
log.
*/
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode_for_socket_access(),
errmsg("could not receive data from
client: %m")));
return EOF;
@@ -1027,7 +1027,7 @@ pq_getbyte_if_available(unsigned char *c)
* cause recursion to here, leading to stack overflow
and core
* dump! This message must go *only* to the postmaster
log.
*/
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode_for_socket_access(),
errmsg("could not receive data from
client: %m")));
r = EOF;
@@ -1238,7 +1238,7 @@ pq_getmessage(StringInfo s, int maxlen)
/* Read message length word */
if (pq_getbytes((char *) &len, 4) == EOF)
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unexpected EOF within message length
word")));
return EOF;
@@ -1249,7 +1249,7 @@ pq_getmessage(StringInfo s, int maxlen)
if (len < 4 ||
(maxlen > 0 && len > maxlen))
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid message length")));
return EOF;
@@ -1271,7 +1271,7 @@ pq_getmessage(StringInfo s, int maxlen)
PG_CATCH();
{
if (pq_discardbytes(len) == EOF)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("incomplete message
from client")));
@@ -1284,7 +1284,7 @@ pq_getmessage(StringInfo s, int maxlen)
/* And grab the message */
if (pq_getbytes(s->data, len) == EOF)
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("incomplete message from
client")));
return EOF;
@@ -1417,7 +1417,7 @@ internal_flush(void)
if (errno != last_reported_send_errno)
{
last_reported_send_errno = errno;
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode_for_socket_access(),
errmsg("could not send data to
client: %m")));
}
diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c
index 350210b..9bdf636 100644
--- a/src/backend/libpq/pqmq.c
+++ b/src/backend/libpq/pqmq.c
@@ -240,7 +240,7 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata)
if (strcmp(value, "DEBUG") == 0)
edata->elevel = DEBUG1; /* or
some other DEBUG level */
else if (strcmp(value, "LOG") == 0)
- edata->elevel = LOG; /*
can't be COMMERROR */
+ edata->elevel = LOG; /*
can't be LOG_SERVER_ONLY */
else if (strcmp(value, "INFO") == 0)
edata->elevel = INFO;
else if (strcmp(value, "NOTICE") == 0)
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 6cf51e1..90571f5 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1889,7 +1889,7 @@ ProcessStartupPacket(Port *port, bool SSLdone)
* don't clutter the log with a complaint.
*/
if (!SSLdone)
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("incomplete startup packet")));
return STATUS_ERROR;
@@ -1901,7 +1901,7 @@ ProcessStartupPacket(Port *port, bool SSLdone)
if (len < (int32) sizeof(ProtocolVersion) ||
len > MAX_STARTUP_PACKET_LENGTH)
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid length of startup packet")));
return STATUS_ERROR;
@@ -1920,7 +1920,7 @@ ProcessStartupPacket(Port *port, bool SSLdone)
if (pq_getbytes(buf, len) == EOF)
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("incomplete startup packet")));
return STATUS_ERROR;
@@ -1959,7 +1959,7 @@ retry1:
{
if (errno == EINTR)
goto retry1; /* if interrupted, just retry */
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode_for_socket_access(),
errmsg("failed to send SSL negotiation
response: %m")));
return STATUS_ERROR; /* close the connection */
diff --git a/src/backend/replication/walsender.c
b/src/backend/replication/walsender.c
index f98475c..211ecb4 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1388,7 +1388,7 @@ ProcessRepliesIfAny(void)
if (r < 0)
{
/* unexpected error or EOF */
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unexpected EOF on standby
connection")));
proc_exit(0);
@@ -1404,7 +1404,7 @@ ProcessRepliesIfAny(void)
resetStringInfo(&reply_message);
if (pq_getmessage(&reply_message, 0))
{
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unexpected EOF on standby
connection")));
proc_exit(0);
@@ -1497,7 +1497,7 @@ ProcessStandbyMessage(void)
break;
default:
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("unexpected message type
\"%c\"", msgtype)));
proc_exit(0);
@@ -1782,7 +1782,7 @@ WalSndCheckTimeOut(TimestampTz now)
* communication problem, we don't send the error message to the
* standby.
*/
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errmsg("terminating walsender process due to replication
timeout")));
WalSndShutdown();
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 68811f1..e4e6dee 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -336,7 +336,7 @@ SocketBackend(StringInfo inBuf)
if (qtype == EOF) /* frontend disconnected */
{
if (IsTransactionState())
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_CONNECTION_FAILURE),
errmsg("unexpected EOF on client
connection with an open transaction")));
else
@@ -372,7 +372,7 @@ SocketBackend(StringInfo inBuf)
if (pq_getstring(inBuf))
{
if (IsTransactionState())
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_CONNECTION_FAILURE),
errmsg("unexpected EOF on client connection with an open transaction")));
else
@@ -399,7 +399,7 @@ SocketBackend(StringInfo inBuf)
if (GetOldFunctionMessage(inBuf))
{
if (IsTransactionState())
- ereport(COMMERROR,
+ ereport(LOG_SERVER_ONLY,
(errcode(ERRCODE_CONNECTION_FAILURE),
errmsg("unexpected EOF on client connection with an open transaction")));
else
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8e00609..740f089 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -293,7 +293,7 @@ errstart(int elevel, const char *filename, int lineno,
output_to_server = is_log_level_output(elevel, log_min_messages);
/* Determine whether message is enabled for client output */
- if (whereToSendOutput == DestRemote && elevel != COMMERROR)
+ if (whereToSendOutput == DestRemote && elevel != LOG_SERVER_ONLY)
{
/*
* client_min_messages is honored only after we complete the
@@ -2086,7 +2086,7 @@ write_eventlog(int level, const char *line, int len)
case DEBUG2:
case DEBUG1:
case LOG:
- case COMMERROR:
+ case LOG_SERVER_ONLY:
case INFO:
case NOTICE:
eventlevel = EVENTLOG_INFORMATION_TYPE;
@@ -2965,7 +2965,7 @@ send_message_to_server_log(ErrorData *edata)
syslog_level = LOG_DEBUG;
break;
case LOG:
- case COMMERROR:
+ case LOG_SERVER_ONLY:
case INFO:
syslog_level = LOG_INFO;
break;
@@ -3595,7 +3595,7 @@ error_severity(int elevel)
prefix = _("DEBUG");
break;
case LOG:
- case COMMERROR:
+ case LOG_SERVER_ONLY:
prefix = _("LOG");
break;
case INFO:
@@ -3699,7 +3699,7 @@ write_stderr(const char *fmt,...)
static bool
is_log_level_output(int elevel, int log_min_level)
{
- if (elevel == LOG || elevel == COMMERROR)
+ if (elevel == LOG || elevel == LOG_SERVER_ONLY)
{
if (log_min_level == LOG || log_min_level <= ERROR)
return true;
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 901651f..69d1140 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -25,9 +25,11 @@
#define DEBUG1 14 /* used by GUC debug_*
variables */
#define LOG 15 /* Server operational
messages; sent only to
* server log
by default. */
-#define COMMERROR 16 /* Client communication
problems; same as LOG
- * for server
reporting, but never sent to
- * client. */
+#define LOG_SERVER_ONLY 16 /* same as LOG for server
reporting, but never
+ sent to
client. */
+#define COMMERROR LOG_SERVER_ONLY /* Client communication
problems;
+
DEPRECATED in favor of
+
LOG_SERVER_ONLY. */
#define INFO 17 /* Messages specifically
requested by user (eg
* VACUUM
VERBOSE output); always sent to
* client
regardless of client_min_messages,
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
