Re: Reducing the log spam
On Thu, May 02, 2024 at 12:47:45PM +0200, Laurenz Albe wrote:
On Mon, 2024-03-11 at 09:33 +0100, Jelte Fennema-Nio wrote:
-�� the subscriber's server log. +�� the subscriber's server log if you remove <literal>23505</literal> from +�� <xref linkend="guc-log-suppress-errcodes"/>.This seems like a pretty big regression. Being able to know why your
replication got closed seems pretty critical.Yes. But I'd argue that that is a shortcoming of logical replication:
there should be a ways to get this information via SQL. Having to look into
the log file is not a very useful option.The feature will become much less useful if unique voilations keep getting logged.
Uh, to be clear, your patch is changing the *defaults*, which I found
surprising, even after reaading the thread. Evidently, the current
behavior is not what you want, and you want to change it, but I'm *sure*
that whatever default you want to use at your site/with your application
is going to make someone else unhappy. I surely want unique violations
to be logged, for example.
@@ -6892,6 +6892,41 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>+ <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes"> + <term><varname>log_suppress_errcodes</varname> (<type>string</type>) + <indexterm> + <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages + from client backend processes with certain error codes to be excluded + from the log. + The value is a comma-separated list of five-character error codes as + listed in <xref linkend="errcodes-appendix"/>. An error code that + represents a class of errors (ends with three zeros) suppresses logging + of all error codes within that class. For example, the entry + <literal>08000</literal> (<literal>connection_exception</literal>) + would suppress an error with code <literal>08P01</literal> + (<literal>protocol_violation</literal>). The default setting is + <literal>23505,3D000,3F000,42601,42704,42883,42P01,57P03</literal>. + Only superusers and users with the appropriate <literal>SET</literal> + privilege can change this setting. + </para>
+ + <para> + This setting is useful to exclude error messages from the log that are + frequent but irrelevant.
I think you should phrase the feature as ".. *allow* skipping error
logging for messages ... that are frequent but irrelevant for a given
site/role/DB/etc."
I suggest that this patch should not change the default behavior at all:
its default should be empty. That you, personally, would plan to
exclude this or that error code is pretty uninteresting. I think the
idea of changing the default behavior will kill the patch, and even if
you want to propose to do that, it should be a separate discussion.
Maybe you should make it an 002 patch.
+ {"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN, + gettext_noop("ERROR and FATAL messages with these error codes don't get logged."), + NULL, + GUC_LIST_INPUT + }, + &log_suppress_errcodes, + DEFAULT_LOG_SUPPRESS_ERRCODES, + check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+/* + * Default value for log_suppress_errcodes. ERROR or FATAL messages with + * these error codes are never logged. Error classes (error codes ending with + * three zeros) match all error codes in the class. The idea is to suppress + * messages that usually don't indicate a serious problem but tend to pollute + * the log file. + */ + +#define DEFAULT_LOG_SUPPRESS_ERRCODES "23505,3D000,3F000,42601,42704,42883,42P01,57P03" +
../src/backend/utils/errcodes.txt:23505 E ERRCODE_UNIQUE_VIOLATION unique_violation
../src/backend/utils/errcodes.txt:3D000 E ERRCODE_INVALID_CATALOG_NAME invalid_catalog_name
../src/backend/utils/errcodes.txt:3F000 E ERRCODE_INVALID_SCHEMA_NAME invalid_schema_name
../src/backend/utils/errcodes.txt:42601 E ERRCODE_SYNTAX_ERROR syntax_error
../src/backend/utils/errcodes.txt:3D000 E ERRCODE_UNDEFINED_DATABASE
../src/backend/utils/errcodes.txt:42883 E ERRCODE_UNDEFINED_FUNCTION undefined_function
../src/backend/utils/errcodes.txt:3F000 E ERRCODE_UNDEFINED_SCHEMA
../src/backend/utils/errcodes.txt:42P01 E ERRCODE_UNDEFINED_TABLE undefined_table
../src/backend/utils/errcodes.txt:42704 E ERRCODE_UNDEFINED_OBJECT undefined_object
../src/backend/utils/errcodes.txt:57P03 E ERRCODE_CANNOT_CONNECT_NOW cannot_connect_now
--
Justin
Import Notes
Reply to msg id not found: a13864771f0193fd19ab017e281cd498f5898380.camel@cybertec.atcf374c4a52d2c93cdc118f4245feb6cc822a34f8.camel@cybertec.at
On Mon, 2024-06-17 at 16:40 -0500, Justin Pryzby wrote:
The feature will become much less useful if unique voilations keep getting logged.
Uh, to be clear, your patch is changing the *defaults*, which I found
surprising, even after reaading the thread. Evidently, the current
behavior is not what you want, and you want to change it, but I'm *sure*
that whatever default you want to use at your site/with your application
is going to make someone else unhappy. I surely want unique violations
to be logged, for example.
I was afraid that setting the default non-empty would cause objections.
+ <para> + This setting is useful to exclude error messages from the log that are + frequent but irrelevant.I think you should phrase the feature as ".. *allow* skipping error
logging for messages ... that are frequent but irrelevant for a given
site/role/DB/etc."
I have reworded that part.
I suggest that this patch should not change the default behavior at all:
its default should be empty. That you, personally, would plan to
exclude this or that error code is pretty uninteresting. I think the
idea of changing the default behavior will kill the patch, and even if
you want to propose to do that, it should be a separate discussion.
Maybe you should make it an 002 patch.
I have attached a new version that leaves the parameter empty by default.
The patch is not motivated by my personal dislike of certain error messages.
A well-written application would not need that parameter at all.
The motivation for me is based on my dealing with customers' log files,
which are often full of messages that are only distracting from serious
problems and fill up the disk.
But you are probably right that it would be hard to find a default setting
that nobody has quibbles with, and the default can always be changed with
a future patch.
Yours,
Laurenz Albe
Attachments:
v4-0001-Add-parameter-log_suppress_errcodes.patchtext/x-patch; charset=UTF-8; name=v4-0001-Add-parameter-log_suppress_errcodes.patchDownload
From 8e1eeb9cbcb94d9b15eb9ee97956cc4044ff7964 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Tue, 18 Jun 2024 18:39:05 +0200
Subject: [PATCH v4] Add parameter log_suppress_errcodes
The parameter contains a list of SQLSTATEs for which
FATAL and ERROR messages are not logged. This is to
suppress messages that are of little interest to the
database administrator, but tend to clutter the log.
Author: Laurenz Albe
Discussion: https://postgr.es/m/408f399e7de1416c47bab7e260327ed5ad92838c.camel%40cybertec.at
---
doc/src/sgml/config.sgml | 35 ++++++
src/backend/utils/error/elog.c | 119 ++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 11 ++
src/backend/utils/misc/postgresql.conf.sample | 3 +
src/include/pg_config_manual.h | 10 ++
src/include/utils/guc.h | 1 +
src/include/utils/guc_hooks.h | 2 +
7 files changed, 181 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 698169afdb..a58b43bceb 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6883,6 +6883,41 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
+ <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
+ from client backend processes with certain error codes to be excluded
+ from the log.
+ The value is a comma-separated list of five-character error codes as
+ listed in <xref linkend="errcodes-appendix"/>. An error code that
+ represents a class of errors (ends with three zeros) suppresses logging
+ of all error codes within that class. For example, the entry
+ <literal>08000</literal> (<literal>connection_exception</literal>)
+ would suppress an error with code <literal>08P01</literal>
+ (<literal>protocol_violation</literal>). The default setting is
+ empty, that is, all error codes get logged.
+ Only superusers and users with the appropriate <literal>SET</literal>
+ privilege can change this setting.
+ </para>
+
+ <para>
+ This setting allows you to skip error logging for messages that are
+ frequent but irrelevant. Supressing such messages makes it easier to
+ spot relevant messages in the log and keeps log files from growing too
+ big. For example, if you have a monitoring system that regularly
+ establishes a TCP connection to the server without sending a correct
+ startup packet, you could suppress the protocol violation errors by
+ adding the error code <literal>08P01</literal> to the list.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-min-duration-statement" xreflabel="log_min_duration_statement">
<term><varname>log_min_duration_statement</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d91a85cb2d..0b2506e07a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -111,12 +111,16 @@ int Log_error_verbosity = PGERROR_DEFAULT;
char *Log_line_prefix = NULL; /* format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+char *log_suppress_errcodes = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
+/* Processed form form of log_suppress_errcodes (zero-terminated array) */
+static int *suppressed_errcodes;
+
#ifdef HAVE_SYSLOG
/*
@@ -863,6 +867,30 @@ errcode(int sqlerrcode)
edata->sqlerrcode = sqlerrcode;
+ /*
+ * ERROR and FATAL messages with codes in log_suppress_errcodes don't get
+ * logged. We only want to suppress error messages from ordinary backend
+ * processes.
+ */
+ if ((MyBackendType == B_BACKEND ||
+ MyBackendType == B_STANDALONE_BACKEND) &&
+ (edata->elevel == ERROR ||
+ edata->elevel == FATAL) &&
+ suppressed_errcodes != NULL)
+ {
+ int *state;
+
+ for (state = suppressed_errcodes; *state != 0; state++)
+ /* error categories match all error codes in the category */
+ if (sqlerrcode == *state ||
+ (ERRCODE_IS_CATEGORY(*state) &&
+ ERRCODE_TO_CATEGORY(sqlerrcode) == *state))
+ {
+ edata->output_to_server = false;
+ break;
+ }
+ }
+
return 0; /* return value does not matter */
}
@@ -2263,6 +2291,97 @@ assign_log_destination(const char *newval, void *extra)
Log_destination = *((int *) extra);
}
+/*
+ * GUC check_hook for log_suppress_errcodes
+ */
+bool
+check_log_suppress_errcodes(char **newval, void **extra, GucSource source)
+{
+ /* SplitIdentifierString modifies the string */
+ char *new_copy = pstrdup(*newval);
+ int listlen;
+ int *statelist = NULL;
+ int index = 0;
+ List *states;
+ ListCell *c;
+
+ if (!SplitIdentifierString(new_copy, ',', &states))
+ {
+ GUC_check_errdetail("List syntax is invalid.");
+ goto failed;
+ }
+
+ listlen = list_length(states);
+ statelist = guc_malloc(ERROR, sizeof(int) * (listlen + 1));
+
+ /* check all error codes for validity and compile them into statelist */
+ foreach(c, states)
+ {
+ char *state = lfirst(c);
+ char *p;
+ int errcode;
+
+ if (strlen(state) != 5)
+ {
+ GUC_check_errdetail("error codes must have 5 characters.");
+ goto failed;
+ }
+
+ /*
+ * Check the the values are alphanumeric and convert them to upper case
+ * (SplitIdentifierString converted them to lower case).
+ */
+ for (p = state; *p != '\0'; p++)
+ if (*p >= 'a' && *p <= 'z')
+ *p += 'A' - 'a';
+ else if (*p < '0' || *p > '9')
+ {
+ GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
+ goto failed;
+ }
+
+ errcode = MAKE_SQLSTATE(state[0], state[1], state[2], state[3], state[4]);
+ /* ignore 0: it cannot be an error code, and we use it to end the list */
+ if (errcode == ERRCODE_SUCCESSFUL_COMPLETION)
+ continue;
+
+ statelist[index++] = errcode;
+ }
+ statelist[index] = 0;
+
+ list_free(states);
+ pfree(new_copy);
+
+ *extra = statelist;
+ return true;
+
+failed:
+ list_free(states);
+ pfree(new_copy);
+ guc_free(statelist);
+ return false;
+}
+
+/*
+ * GUC assign_hook for log_suppress_errcodes
+ */
+void
+assign_log_suppress_errcodes(const char *newval, void *extra)
+{
+ /* free the memory for the old entries */
+ if (suppressed_errcodes != NULL)
+ pfree(suppressed_errcodes);
+
+ /* store NULL instead of an empty array for performance */
+ if (*(int *)extra == 0)
+ {
+ guc_free(extra);
+ suppressed_errcodes = NULL;
+ }
+ else
+ suppressed_errcodes = extra;
+}
+
/*
* GUC assign_hook for syslog_ident
*/
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 46c258be28..0341c805b9 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4487,6 +4487,17 @@ struct config_string ConfigureNamesString[] =
check_canonical_path, NULL, NULL
},
+ {
+ {"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
+ NULL,
+ GUC_LIST_INPUT
+ },
+ &log_suppress_errcodes,
+ DEFAULT_LOG_SUPPRESS_ERRCODES,
+ check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+ },
+
{
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e0567de219..eefeeff487 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -539,6 +539,9 @@
# fatal
# panic (effectively off)
+#log_suppress_errcodes = '' # FATAL and ERROR messages with these error codes
+ # are not logged
+
#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
# and their durations, > 0 logs only
# statements running at least this number
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index f941ee2faf..8c71bc2833 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -216,6 +216,16 @@
*/
#define DEFAULT_EVENT_SOURCE "PostgreSQL"
+/*
+ * Default value for log_suppress_errcodes. ERROR or FATAL messages with
+ * these error codes are never logged. Error classes (error codes ending with
+ * three zeros) match all error codes in the class. The idea is to suppress
+ * messages that usually don't indicate a serious problem but tend to pollute
+ * the log file.
+ */
+
+#define DEFAULT_LOG_SUPPRESS_ERRCODES ""
+
/*
* Assumed cache line size. This doesn't affect correctness, but can be used
* for low-level optimizations. This is mostly used to pad various data
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e4a594b5e8..d3ac962fa7 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -259,6 +259,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
+extern PGDLLIMPORT char *log_suppress_errcodes;
extern PGDLLIMPORT int log_min_messages;
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index d64dc5fcdb..3b312e78fb 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -78,6 +78,8 @@ extern bool check_log_destination(char **newval, void **extra,
extern void assign_log_destination(const char *newval, void *extra);
extern const char *show_log_file_mode(void);
extern bool check_log_stats(bool *newval, void **extra, GucSource source);
+extern bool check_log_suppress_errcodes(char **newval, void **extra, GucSource source);
+extern void assign_log_suppress_errcodes(const char *newval, void *extra);
extern bool check_log_timezone(char **newval, void **extra, GucSource source);
extern void assign_log_timezone(const char *newval, void *extra);
extern const char *show_log_timezone(void);
--
2.45.1
Hello Laurenz,
I liked the idea for this patch. I will also go for the default being
an empty string.
I went through this patch and have some comments on the code,
1. In general, I don't like the idea of goto, maybe we can have a
free_something function to call here.
2.
if (!SplitIdentifierString(new_copy, ',', &states))
{
GUC_check_errdetail("List syntax is invalid.");
goto failed;
}
Here, we don't need all that free-ing, we can just return false here.
3.
/*
* Check the the values are alphanumeric and convert them to upper case
* (SplitIdentifierString converted them to lower case).
*/
for (p = state; *p != '\0'; p++)
if (*p >= 'a' && *p <= 'z')
*p += 'A' - 'a';
else if (*p < '0' || *p > '9')
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
I was thinking, maybe we can use tolower() function here.
4.
list_free(states);
pfree(new_copy);
*extra = statelist;
return true;
failed:
list_free(states);
pfree(new_copy);
guc_free(statelist);
return false;
This looks like duplication that can be easily avoided.
You may have free_somthing function to do all free-ing stuff only and
its callee can then have a return statement.
e.g for here,
free_states(states, new_copy, statelist);
return true;
5. Also, for alphanumeric check, maybe we can have something like,
if (isalnum(*state) == 0)
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
and we can do this in the beginning after the len check.
On Tue, 18 Jun 2024 at 18:49, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
On Mon, 2024-06-17 at 16:40 -0500, Justin Pryzby wrote:
The feature will become much less useful if unique voilations keep getting logged.
Uh, to be clear, your patch is changing the *defaults*, which I found
surprising, even after reaading the thread. Evidently, the current
behavior is not what you want, and you want to change it, but I'm *sure*
that whatever default you want to use at your site/with your application
is going to make someone else unhappy. I surely want unique violations
to be logged, for example.I was afraid that setting the default non-empty would cause objections.
+ <para> + This setting is useful to exclude error messages from the log that are + frequent but irrelevant.I think you should phrase the feature as ".. *allow* skipping error
logging for messages ... that are frequent but irrelevant for a given
site/role/DB/etc."I have reworded that part.
I suggest that this patch should not change the default behavior at all:
its default should be empty. That you, personally, would plan to
exclude this or that error code is pretty uninteresting. I think the
idea of changing the default behavior will kill the patch, and even if
you want to propose to do that, it should be a separate discussion.
Maybe you should make it an 002 patch.I have attached a new version that leaves the parameter empty by default.
The patch is not motivated by my personal dislike of certain error messages.
A well-written application would not need that parameter at all.
The motivation for me is based on my dealing with customers' log files,
which are often full of messages that are only distracting from serious
problems and fill up the disk.But you are probably right that it would be hard to find a default setting
that nobody has quibbles with, and the default can always be changed with
a future patch.Yours,
Laurenz Albe
--
Regards,
Rafia Sabih
Thanks for the review!
On Wed, 2024-07-24 at 15:27 +0200, Rafia Sabih wrote:
I liked the idea for this patch. I will also go for the default being
an empty string.
I went through this patch and have some comments on the code,1. In general, I don't like the idea of goto, maybe we can have a
free_something function to call here.
The PostgreSQL code base has over 3000 goto's...
Sure, that can be factored out to a function (except the final "return"),
but I feel that a function for three "free" calls is code bloat.
Do you think that avoiding the goto and using a function would make the
code simpler and clearer?
2.
if (!SplitIdentifierString(new_copy, ',', &states))
{
GUC_check_errdetail("List syntax is invalid.");
goto failed;
}
Here, we don't need all that free-ing, we can just return false here.
I am OK with changing that; I had thought it was more clearer and more
foolproof to use the same pattern everywhere.
3.
/*
* Check the the values are alphanumeric and convert them to upper case
* (SplitIdentifierString converted them to lower case).
*/
for (p = state; *p != '\0'; p++)
if (*p >= 'a' && *p <= 'z')
*p += 'A' - 'a';
else if (*p < '0' || *p > '9')
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
I was thinking, maybe we can use tolower() function here.
That is a good idea, but these C library respect the current locale.
I would have to explicitly specify the C locale or switch the locale
temporarily.
Switching the locale seems clumsy, and I have no idea what I would have
to feed as second argument to toupper_l() or isalnum_l().
Do you have an idea?
4.
list_free(states);
pfree(new_copy);*extra = statelist;
return true;failed:
list_free(states);
pfree(new_copy);
guc_free(statelist);
return false;This looks like duplication that can be easily avoided.
You may have free_somthing function to do all free-ing stuff only and
its callee can then have a return statement.
e.g for here,
free_states(states, new_copy, statelist);
return true;
That free_states() function would just contain two function calls.
I think that defining a special function for that is somewhat out of
proportion.
5. Also, for alphanumeric check, maybe we can have something like,
if (isalnum(*state) == 0)
{
GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
goto failed;
}
and we can do this in the beginning after the len check.
isalnum() operates on a single character and depends on the current locale.
See my comments to 3. above.
Please let me know what you think, particularly about the locale problem.
Yours,
Laurenz Albe
On Thu, 25 Jul 2024 at 18:03, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
Thanks for the review!
On Wed, 2024-07-24 at 15:27 +0200, Rafia Sabih wrote:
I liked the idea for this patch. I will also go for the default being
an empty string.
I went through this patch and have some comments on the code,1. In general, I don't like the idea of goto, maybe we can have a
free_something function to call here.The PostgreSQL code base has over 3000 goto's...
Sure, that can be factored out to a function (except the final "return"),
but I feel that a function for three "free" calls is code bloat.On a detailed look over this, you are right Laurenz about this.
Do you think that avoiding the goto and using a function would make the
code simpler and clearer?2.
if (!SplitIdentifierString(new_copy, ',', &states))
{
GUC_check_errdetail("List syntax is invalid.");
goto failed;
}
Here, we don't need all that free-ing, we can just return false here.I am OK with changing that; I had thought it was more clearer and more
foolproof to use the same pattern everywhere.3.
/*
* Check the the values are alphanumeric and convert them to upper case
* (SplitIdentifierString converted them to lower case).
*/
for (p = state; *p != '\0'; p++)
if (*p >= 'a' && *p <= 'z')
*p += 'A' - 'a';
else if (*p < '0' || *p > '9')
{
GUC_check_errdetail("error codes can only contain digits and ASCIIletters.");
goto failed;
}
I was thinking, maybe we can use tolower() function here.That is a good idea, but these C library respect the current locale.
I would have to explicitly specify the C locale or switch the locale
temporarily.
Hmm. actually I don't have any good answers to this locale issue.
Switching the locale seems clumsy, and I have no idea what I would have
to feed as second argument to toupper_l() or isalnum_l().
Do you have an idea?4.
list_free(states);
pfree(new_copy);*extra = statelist;
return true;failed:
list_free(states);
pfree(new_copy);
guc_free(statelist);
return false;This looks like duplication that can be easily avoided.
You may have free_somthing function to do all free-ing stuff only and
its callee can then have a return statement.
e.g for here,
free_states(states, new_copy, statelist);
return true;That free_states() function would just contain two function calls.
I think that defining a special function for that is somewhat out of
proportion.5. Also, for alphanumeric check, maybe we can have something like,
if (isalnum(*state) == 0)
{
GUC_check_errdetail("error codes can only contain digits and ASCIIletters.");
goto failed;
}
and we can do this in the beginning after the len check.isalnum() operates on a single character and depends on the current locale.
See my comments to 3. above.Please let me know what you think, particularly about the locale problem.
Yours,
Laurenz Albe
--
Regards,
Rafia Sabih
Hi Laurenz
On 18.06.24 18:49, Laurenz Albe wrote:
I have attached a new version that leaves the parameter empty by default.
I've tested this patch and for the most part it works as intended. For
convenience, I wrote a small function to simulate the exceptions using a
given errcode:
CREATE OR REPLACE FUNCTION raise_sqlstate(state text)
RETURNS VOID AS $$
BEGIN
RAISE EXCEPTION 'exception sqlstate = %',
state USING ERRCODE = state;
END;
$$ LANGUAGE plpgsql;
== specific errcodes ==
SET log_suppress_errcodes TO '23505,3D000,42601';
SHOW log_suppress_errcodes;
SELECT raise_sqlstate('23505'); -- must be suppressed
SELECT raise_sqlstate('3D000'); -- must be suppressed
SELECT raise_sqlstate('42601'); -- must be suppressed
SELECT raise_sqlstate('42P01');
log entries:
2025-03-07 14:23:30.688 CET [3266008] ERROR: exception sqlstate = 42P01
2025-03-07 14:23:30.688 CET [3266008] CONTEXT: PL/pgSQL function
raise_sqlstate(text) line 3 at RAISE
2025-03-07 14:23:30.688 CET [3266008] STATEMENT: SELECT
raise_sqlstate('42P01');
== errcode classes ==
SET log_suppress_errcodes TO '23000,3D000';
SHOW log_suppress_errcodes;
SELECT raise_sqlstate('23505'); -- must be suppressed
SELECT raise_sqlstate('42P01');
log entries:
2025-03-07 14:24:40.023 CET [3268343] ERROR: exception sqlstate = 42P01
2025-03-07 14:24:40.023 CET [3268343] CONTEXT: PL/pgSQL function
raise_sqlstate(text) line 3 at RAISE
2025-03-07 14:24:40.023 CET [3268343] STATEMENT: SELECT
raise_sqlstate('42P01');
There are a few issues though ...
1) Invalid codes aren't rejected. Is there any way to validate them?
postgres=# SET log_suppress_errcodes TO '0foo1'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
0foo1
(1 row)
2) No duplication check (not critical IMO)
postgres=# SET log_suppress_errcodes TO '3F000,3F000'; SHOW
log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,3F000
(1 row)
3) errcodes are not trimmed (also not critical.. just doesn't look nice)
postgres=# SET log_suppress_errcodes TO ' 3F000, 42P01';
SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------------
3F000, 42P01
(1 row)
4) SHOW log_suppress_errcodes displays an invalid value if we set it
twice to an empty string
$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.
postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
(1 row)
SET
log_suppress_errcodes
-----------------------
wV
(1 row)
5) The system crashes if we set log_suppress_errcodesto an empty string
and then set it back to comma separated values
$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.
postgres=# SET log_suppress_errcodes TO '3F000,42883'; SHOW
log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '42P01,23505'; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '3D000,42704'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,42883
(1 row)
SET
log_suppress_errcodes
-----------------------
(1 row)
SET
log_suppress_errcodes
-----------------------
42P01,23505
(1 row)
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
The connection to the server was lost. Attempting reset: Failed.
You are currently not connected to a database.
!?>
Best regards, Jim
On Fri, 2025-03-07 at 20:38 +0100, Jim Jones wrote:
I've tested this patch and for the most part it works as intended.
Thanks for the thorough test!
There are a few issues though ...
1) Invalid codes aren't rejected. Is there any way to validate them?
postgres=# SET log_suppress_errcodes TO '0foo1'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
0foo1
(1 row)
That is intentional. I only test that the SQLSTATE is 5 characters long
and contains only ASCII letters and numbers. I looked at the SQL standard,
and while it defines the meaning of certain SQLSTATEs, it allows for
custom defined states.
Nothing bad can happen from setting an unused SQLSTATE; it just won't
suppress anything.
2) No duplication check (not critical IMO)
postgres=# SET log_suppress_errcodes TO '3F000,3F000'; SHOW
log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,3F000
(1 row)3) errcodes are not trimmed (also not critical.. just doesn't look nice)
postgres=# SET log_suppress_errcodes TO ' 3F000, 42P01';
SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------------
3F000, 42P01
(1 row)
These two are mostly cosmetic issues.
I originally thought it best to leave the parameter the way the user
entered it, but in the attached version I revised that by reassembling
the parameter string from the parsed SQLSTATEs, so that spaces and
duplicates will vanish and everything is converted to upper case.
So these complaints should be addressed.
4) SHOW log_suppress_errcodes displays an invalid value if we set it
twice to an empty string$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
(1 row)SET
log_suppress_errcodes
-----------------------
wV
(1 row)5) The system crashes if we set log_suppress_errcodesto an empty string
and then set it back to comma separated values
These two points were actually caused by a memory management bug that I
had inadvertently introduced in the assign hook. They should be fixed now.
Attached is the fifth version of the patch.
Yours,
Laurenz Albe
Attachments:
v5-0001-Add-parameter-log_suppress_errcodes.patchtext/x-patch; charset=UTF-8; name=v5-0001-Add-parameter-log_suppress_errcodes.patchDownload
From 5885419838597293ac18b131017f8f778261f3d9 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Tue, 11 Mar 2025 10:32:33 +0100
Subject: [PATCH v5] Add parameter log_suppress_errcodes
The parameter contains a list of SQLSTATEs for which
FATAL and ERROR messages are not logged. This is to
suppress messages that are of little interest to the
database administrator, but tend to clutter the log.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Discussion: https://postgr.es/m/408f399e7de1416c47bab7e260327ed5ad92838c.camel%40cybertec.at
---
doc/src/sgml/config.sgml | 35 ++++
src/backend/utils/error/elog.c | 149 ++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 11 ++
src/backend/utils/misc/postgresql.conf.sample | 3 +
src/include/pg_config_manual.h | 10 ++
src/include/utils/guc.h | 1 +
src/include/utils/guc_hooks.h | 2 +
7 files changed, 211 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d2fa5f7d1a9..ba5f60977a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6916,6 +6916,41 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
+ <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
+ from client backend processes with certain error codes to be excluded
+ from the log.
+ The value is a comma-separated list of five-character error codes as
+ listed in <xref linkend="errcodes-appendix"/>. An error code that
+ represents a class of errors (ends with three zeros) suppresses logging
+ of all error codes within that class. For example, the entry
+ <literal>08000</literal> (<literal>connection_exception</literal>)
+ would suppress an error with code <literal>08P01</literal>
+ (<literal>protocol_violation</literal>). The default setting is
+ empty, that is, all error codes get logged.
+ Only superusers and users with the appropriate <literal>SET</literal>
+ privilege can change this setting.
+ </para>
+
+ <para>
+ This setting allows you to skip error logging for messages that are
+ frequent but irrelevant. Supressing such messages makes it easier to
+ spot relevant messages in the log and keeps log files from growing too
+ big. For example, if you have a monitoring system that regularly
+ establishes a TCP connection to the server without sending a correct
+ startup packet, you could suppress the protocol violation errors by
+ adding the error code <literal>08P01</literal> to the list.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-min-duration-statement" xreflabel="log_min_duration_statement">
<term><varname>log_min_duration_statement</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42..fc66f6ee089 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -109,12 +109,16 @@ int Log_error_verbosity = PGERROR_DEFAULT;
char *Log_line_prefix = NULL; /* format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+char *log_suppress_errcodes = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
+/* Processed form form of log_suppress_errcodes (zero-terminated array) */
+static int *suppressed_errcodes;
+
#ifdef HAVE_SYSLOG
/*
@@ -859,6 +863,30 @@ errcode(int sqlerrcode)
edata->sqlerrcode = sqlerrcode;
+ /*
+ * ERROR and FATAL messages with codes in log_suppress_errcodes don't get
+ * logged. We only want to suppress error messages from ordinary backend
+ * processes.
+ */
+ if ((MyBackendType == B_BACKEND ||
+ MyBackendType == B_STANDALONE_BACKEND) &&
+ (edata->elevel == ERROR ||
+ edata->elevel == FATAL) &&
+ suppressed_errcodes != NULL)
+ {
+ int *state;
+
+ for (state = suppressed_errcodes; *state != 0; state++)
+ /* error categories match all error codes in the category */
+ if (sqlerrcode == *state ||
+ (ERRCODE_IS_CATEGORY(*state) &&
+ ERRCODE_TO_CATEGORY(sqlerrcode) == *state))
+ {
+ edata->output_to_server = false;
+ break;
+ }
+ }
+
return 0; /* return value does not matter */
}
@@ -2299,6 +2327,127 @@ assign_log_destination(const char *newval, void *extra)
Log_destination = *((int *) extra);
}
+/*
+ * GUC check_hook for log_suppress_errcodes
+ *
+ * Split the string on the commas, check the SQLSTATEs for validity, convert
+ * them into the packed integer form and store them in a 0-terminated array.
+ * That array is returned as "extra" and will be assigned to
+ * "suppressed_errcodes" in the assign hook.
+ *
+ * Replace the actual parameter with a sanitized version reassembled from that
+ * array.
+ */
+bool
+check_log_suppress_errcodes(char **newval, void **extra, GucSource source)
+{
+ /* SplitIdentifierString modifies the string */
+ char *new_copy = pstrdup(*newval);
+ int listlen;
+ int *statelist = NULL;
+ int index = 0;
+ int param_len = 1;
+ List *states;
+ ListCell *c;
+
+ if (!SplitIdentifierString(new_copy, ',', &states))
+ {
+ GUC_check_errdetail("List syntax is invalid.");
+ goto failed;
+ }
+
+ listlen = list_length(states);
+ statelist = guc_malloc(ERROR, sizeof(int) * (listlen + 1));
+
+ /* check all error codes for validity and compile them into statelist */
+ foreach(c, states)
+ {
+ char *state = lfirst(c);
+ char *p;
+ int errcode, i;
+ bool duplicate = false;
+
+ if (strlen(state) != 5)
+ {
+ GUC_check_errdetail("error codes must have 5 characters.");
+ goto failed;
+ }
+
+ /*
+ * Check the the values are alphanumeric and convert them to upper case
+ * (SplitIdentifierString converted them to lower case).
+ */
+ for (p = state; *p != '\0'; p++)
+ if (*p >= 'a' && *p <= 'z')
+ *p += 'A' - 'a';
+ else if (*p < '0' || *p > '9')
+ {
+ GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
+ goto failed;
+ }
+
+ errcode = MAKE_SQLSTATE(state[0], state[1], state[2], state[3], state[4]);
+ /* ignore 0: it cannot be an error code, and we use it to end the list */
+ if (errcode == ERRCODE_SUCCESSFUL_COMPLETION)
+ continue;
+
+ /* ignore duplicate entries */
+ for (i = 0; i < index; i++)
+ if (statelist[i] == errcode)
+ duplicate = true;
+ if (duplicate)
+ continue;
+
+ statelist[index++] = errcode;
+ param_len += 6;
+ }
+ statelist[index] = 0;
+
+ list_free(states);
+ pfree(new_copy);
+
+ *extra = statelist;
+
+ /*
+ * Now that we have successfully parsed the SQLSTATEs, reassemble a string
+ * list for the actual parameter value. That will remove extra spaces and
+ * duplicates and convert the values to upper case.
+ */
+ guc_free(*newval);
+ *newval = guc_malloc(ERROR, param_len);
+ (*newval)[0] = '\0';
+
+ index = 0;
+ while (statelist[index] != 0)
+ {
+ if (index > 0)
+ strncat(*newval, ",", 2);
+ strncat(*newval, unpack_sql_state(statelist[index]), 6);
+ index++;
+ }
+
+ return true;
+
+failed:
+ list_free(states);
+ pfree(new_copy);
+ guc_free(statelist); /* won't gag on NULL */
+ return false;
+}
+
+/*
+ * GUC assign_hook for log_suppress_errcodes
+ */
+void
+assign_log_suppress_errcodes(const char *newval, void *extra)
+{
+ /* store NULL instead of an empty array for performance */
+ if (*(int *)extra == 0)
+ suppressed_errcodes = NULL;
+ else
+ suppressed_errcodes = extra;
+}
+
/*
* GUC assign_hook for syslog_ident
*/
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..02f0570ef2f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4635,6 +4635,17 @@ struct config_string ConfigureNamesString[] =
check_canonical_path, NULL, NULL
},
+ {
+ {"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
+ NULL,
+ GUC_LIST_INPUT
+ },
+ &log_suppress_errcodes,
+ DEFAULT_LOG_SUPPRESS_ERRCODES,
+ check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+ },
+
{
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2d1de9c37bd..24fa9163fee 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -541,6 +541,9 @@
# fatal
# panic (effectively off)
+#log_suppress_errcodes = '' # FATAL and ERROR messages with these error codes
+ # are not logged
+
#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
# and their durations, > 0 logs only
# statements running at least this number
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 449e50bd78c..7fc90ea18c4 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -200,6 +200,16 @@
*/
#define DEFAULT_EVENT_SOURCE "PostgreSQL"
+/*
+ * Default value for log_suppress_errcodes. ERROR or FATAL messages with
+ * these error codes are never logged. Error classes (error codes ending with
+ * three zeros) match all error codes in the class. The idea is to suppress
+ * messages that usually don't indicate a serious problem but tend to pollute
+ * the log file.
+ */
+
+#define DEFAULT_LOG_SUPPRESS_ERRCODES ""
+
/*
* Assumed cache line size. This doesn't affect correctness, but can be used
* for low-level optimizations. This is mostly used to pad various data
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1233e07d7da..4f29133ae7c 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -271,6 +271,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
+extern PGDLLIMPORT char *log_suppress_errcodes;
extern PGDLLIMPORT int log_min_messages;
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 951451a9765..e55803a0fea 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -76,6 +76,8 @@ extern bool check_log_destination(char **newval, void **extra,
extern void assign_log_destination(const char *newval, void *extra);
extern const char *show_log_file_mode(void);
extern bool check_log_stats(bool *newval, void **extra, GucSource source);
+extern bool check_log_suppress_errcodes(char **newval, void **extra, GucSource source);
+extern void assign_log_suppress_errcodes(const char *newval, void *extra);
extern bool check_log_timezone(char **newval, void **extra, GucSource source);
extern void assign_log_timezone(const char *newval, void *extra);
extern const char *show_log_timezone(void);
--
2.48.1
On 11.03.25 10:46, Laurenz Albe wrote:
Thanks for the thorough test!
There are a few issues though ...
1) Invalid codes aren't rejected. Is there any way to validate them?
postgres=# SET log_suppress_errcodes TO '0foo1'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
0foo1
(1 row)That is intentional. I only test that the SQLSTATE is 5 characters long
and contains only ASCII letters and numbers. I looked at the SQL standard,
and while it defines the meaning of certain SQLSTATEs, it allows for
custom defined states.Nothing bad can happen from setting an unused SQLSTATE; it just won't
suppress anything.
I suspected that. Just for the records, v5 now shows also "invalid"
SQLSTATEs in uppercase:
postgres=# SET log_suppress_errcodes TO 'x1y2z'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
X1Y2Z
(1 row)
2) No duplication check (not critical IMO)
postgres=# SET log_suppress_errcodes TO '3F000,3F000'; SHOW
log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,3F000
(1 row)3) errcodes are not trimmed (also not critical.. just doesn't look nice)
postgres=# SET log_suppress_errcodes TO ' 3F000, 42P01';
SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------------
3F000, 42P01
(1 row)These two are mostly cosmetic issues.
I originally thought it best to leave the parameter the way the user
entered it, but in the attached version I revised that by reassembling
the parameter string from the parsed SQLSTATEs, so that spaces and
duplicates will vanish and everything is converted to upper case.So these complaints should be addressed.
Duplicated entries and whitespaces are now removed
postgres=# SET log_suppress_errcodes TO ' 3F000, 3F000, 42P01';
SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,42P01
(1 row)
4) SHOW log_suppress_errcodes displays an invalid value if we set it
twice to an empty string$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
(1 row)SET
log_suppress_errcodes
-----------------------
wV
(1 row)5) The system crashes if we set log_suppress_errcodesto an empty string
and then set it back to comma separated valuesThese two points were actually caused by a memory management bug that I
had inadvertently introduced in the assign hook. They should be fixed now.
empty log_suppress_errcode now behaves as expected
$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.
postgres=# SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
(1 row)
SET
log_suppress_errcodes
-----------------------
(1 row)
The system no longer crashes when setting the parameter to an empty
string and set it back to comma separated values
$ /usr/local/postgres-dev/bin/psql postgres
psql (18devel)
Type "help" for help.
postgres=# SET log_suppress_errcodes TO '3F000,42883'; SHOW
log_suppress_errcodes;
SET log_suppress_errcodes TO ''; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '42P01,23505'; SHOW log_suppress_errcodes;
SET log_suppress_errcodes TO '3D000,42704'; SHOW log_suppress_errcodes;
SET
log_suppress_errcodes
-----------------------
3F000,42883
(1 row)
SET
log_suppress_errcodes
-----------------------
(1 row)
SET
log_suppress_errcodes
-----------------------
42P01,23505
(1 row)
SET
log_suppress_errcodes
-----------------------
3D000,42704
(1 row)
The previous tests work as expected and all issues were addressed.
If the other reviewers have no objections, I'll mark this patch as
"Ready for Committer".
Best regards, Jim
On Tue, 2025-03-11 at 10:46 +0100, Laurenz Albe wrote:
Attached is the fifth version of the patch.
... and here, for good measure, a pgindented version
Apart from that, no changes.
Yours,
Laurenz Albe
Attachments:
v6-0001-Add-parameter-log_suppress_errcodes.patchtext/x-patch; charset=UTF-8; name=v6-0001-Add-parameter-log_suppress_errcodes.patchDownload
From d644d3fe1b2c5ab2cbf7f10e9c653b7baa9efe01 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 14 Mar 2025 06:56:14 +0100
Subject: [PATCH v6] Add parameter log_suppress_errcodes
The parameter contains a list of SQLSTATEs for which
FATAL and ERROR messages are not logged. This is to
suppress messages that are of little interest to the
database administrator, but tend to clutter the log.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Discussion: https://postgr.es/m/408f399e7de1416c47bab7e260327ed5ad92838c.camel%40cybertec.at
---
doc/src/sgml/config.sgml | 35 ++++
src/backend/utils/error/elog.c | 150 ++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 11 ++
src/backend/utils/misc/postgresql.conf.sample | 3 +
src/include/pg_config_manual.h | 10 ++
src/include/utils/guc.h | 1 +
src/include/utils/guc_hooks.h | 2 +
7 files changed, 212 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d2fa5f7d1a9..ba5f60977a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6916,6 +6916,41 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
+ <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
+ from client backend processes with certain error codes to be excluded
+ from the log.
+ The value is a comma-separated list of five-character error codes as
+ listed in <xref linkend="errcodes-appendix"/>. An error code that
+ represents a class of errors (ends with three zeros) suppresses logging
+ of all error codes within that class. For example, the entry
+ <literal>08000</literal> (<literal>connection_exception</literal>)
+ would suppress an error with code <literal>08P01</literal>
+ (<literal>protocol_violation</literal>). The default setting is
+ empty, that is, all error codes get logged.
+ Only superusers and users with the appropriate <literal>SET</literal>
+ privilege can change this setting.
+ </para>
+
+ <para>
+ This setting allows you to skip error logging for messages that are
+ frequent but irrelevant. Supressing such messages makes it easier to
+ spot relevant messages in the log and keeps log files from growing too
+ big. For example, if you have a monitoring system that regularly
+ establishes a TCP connection to the server without sending a correct
+ startup packet, you could suppress the protocol violation errors by
+ adding the error code <literal>08P01</literal> to the list.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-min-duration-statement" xreflabel="log_min_duration_statement">
<term><varname>log_min_duration_statement</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42..bdcc7d1bd92 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -109,12 +109,16 @@ int Log_error_verbosity = PGERROR_DEFAULT;
char *Log_line_prefix = NULL; /* format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+char *log_suppress_errcodes = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
+/* Processed form form of log_suppress_errcodes (zero-terminated array) */
+static int *suppressed_errcodes;
+
#ifdef HAVE_SYSLOG
/*
@@ -859,6 +863,30 @@ errcode(int sqlerrcode)
edata->sqlerrcode = sqlerrcode;
+ /*
+ * ERROR and FATAL messages with codes in log_suppress_errcodes don't get
+ * logged. We only want to suppress error messages from ordinary backend
+ * processes.
+ */
+ if ((MyBackendType == B_BACKEND ||
+ MyBackendType == B_STANDALONE_BACKEND) &&
+ (edata->elevel == ERROR ||
+ edata->elevel == FATAL) &&
+ suppressed_errcodes != NULL)
+ {
+ int *state;
+
+ for (state = suppressed_errcodes; *state != 0; state++)
+ /* error categories match all error codes in the category */
+ if (sqlerrcode == *state ||
+ (ERRCODE_IS_CATEGORY(*state) &&
+ ERRCODE_TO_CATEGORY(sqlerrcode) == *state))
+ {
+ edata->output_to_server = false;
+ break;
+ }
+ }
+
return 0; /* return value does not matter */
}
@@ -2299,6 +2327,128 @@ assign_log_destination(const char *newval, void *extra)
Log_destination = *((int *) extra);
}
+/*
+ * GUC check_hook for log_suppress_errcodes
+ *
+ * Split the string on the commas, check the SQLSTATEs for validity, convert
+ * them into the packed integer form and store them in a 0-terminated array.
+ * That array is returned as "extra" and will be assigned to
+ * "suppressed_errcodes" in the assign hook.
+ *
+ * Replace the actual parameter with a sanitized version reassembled from that
+ * array.
+ */
+bool
+check_log_suppress_errcodes(char **newval, void **extra, GucSource source)
+{
+ /* SplitIdentifierString modifies the string */
+ char *new_copy = pstrdup(*newval);
+ int listlen;
+ int *statelist = NULL;
+ int index = 0;
+ int param_len = 1;
+ List *states;
+ ListCell *c;
+
+ if (!SplitIdentifierString(new_copy, ',', &states))
+ {
+ GUC_check_errdetail("List syntax is invalid.");
+ goto failed;
+ }
+
+ listlen = list_length(states);
+ statelist = guc_malloc(ERROR, sizeof(int) * (listlen + 1));
+
+ /* check all error codes for validity and compile them into statelist */
+ foreach(c, states)
+ {
+ char *state = lfirst(c);
+ char *p;
+ int errcode,
+ i;
+ bool duplicate = false;
+
+ if (strlen(state) != 5)
+ {
+ GUC_check_errdetail("error codes must have 5 characters.");
+ goto failed;
+ }
+
+ /*
+ * Check the the values are alphanumeric and convert them to upper
+ * case (SplitIdentifierString converted them to lower case).
+ */
+ for (p = state; *p != '\0'; p++)
+ if (*p >= 'a' && *p <= 'z')
+ *p += 'A' - 'a';
+ else if (*p < '0' || *p > '9')
+ {
+ GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
+ goto failed;
+ }
+
+ errcode = MAKE_SQLSTATE(state[0], state[1], state[2], state[3], state[4]);
+ /* ignore 0: it cannot be an error code, and we use it to end the list */
+ if (errcode == ERRCODE_SUCCESSFUL_COMPLETION)
+ continue;
+
+ /* ignore duplicate entries */
+ for (i = 0; i < index; i++)
+ if (statelist[i] == errcode)
+ duplicate = true;
+ if (duplicate)
+ continue;
+
+ statelist[index++] = errcode;
+ param_len += 6;
+ }
+ statelist[index] = 0;
+
+ list_free(states);
+ pfree(new_copy);
+
+ *extra = statelist;
+
+ /*
+ * Now that we have successfully parsed the SQLSTATEs, reassemble a string
+ * list for the actual parameter value. That will remove extra spaces and
+ * duplicates and convert the values to upper case.
+ */
+ guc_free(*newval);
+ *newval = guc_malloc(ERROR, param_len);
+ (*newval)[0] = '\0';
+
+ index = 0;
+ while (statelist[index] != 0)
+ {
+ if (index > 0)
+ strncat(*newval, ",", 2);
+ strncat(*newval, unpack_sql_state(statelist[index]), 6);
+ index++;
+ }
+
+ return true;
+
+failed:
+ list_free(states);
+ pfree(new_copy);
+ guc_free(statelist); /* won't gag on NULL */
+ return false;
+}
+
+/*
+ * GUC assign_hook for log_suppress_errcodes
+ */
+void
+assign_log_suppress_errcodes(const char *newval, void *extra)
+{
+ /* store NULL instead of an empty array for performance */
+ if (*(int *) extra == 0)
+ suppressed_errcodes = NULL;
+ else
+ suppressed_errcodes = extra;
+}
+
/*
* GUC assign_hook for syslog_ident
*/
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..02f0570ef2f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4635,6 +4635,17 @@ struct config_string ConfigureNamesString[] =
check_canonical_path, NULL, NULL
},
+ {
+ {"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
+ NULL,
+ GUC_LIST_INPUT
+ },
+ &log_suppress_errcodes,
+ DEFAULT_LOG_SUPPRESS_ERRCODES,
+ check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+ },
+
{
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2d1de9c37bd..24fa9163fee 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -541,6 +541,9 @@
# fatal
# panic (effectively off)
+#log_suppress_errcodes = '' # FATAL and ERROR messages with these error codes
+ # are not logged
+
#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
# and their durations, > 0 logs only
# statements running at least this number
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 449e50bd78c..7fc90ea18c4 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -200,6 +200,16 @@
*/
#define DEFAULT_EVENT_SOURCE "PostgreSQL"
+/*
+ * Default value for log_suppress_errcodes. ERROR or FATAL messages with
+ * these error codes are never logged. Error classes (error codes ending with
+ * three zeros) match all error codes in the class. The idea is to suppress
+ * messages that usually don't indicate a serious problem but tend to pollute
+ * the log file.
+ */
+
+#define DEFAULT_LOG_SUPPRESS_ERRCODES ""
+
/*
* Assumed cache line size. This doesn't affect correctness, but can be used
* for low-level optimizations. This is mostly used to pad various data
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1233e07d7da..4f29133ae7c 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -271,6 +271,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
+extern PGDLLIMPORT char *log_suppress_errcodes;
extern PGDLLIMPORT int log_min_messages;
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 951451a9765..e55803a0fea 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -76,6 +76,8 @@ extern bool check_log_destination(char **newval, void **extra,
extern void assign_log_destination(const char *newval, void *extra);
extern const char *show_log_file_mode(void);
extern bool check_log_stats(bool *newval, void **extra, GucSource source);
+extern bool check_log_suppress_errcodes(char **newval, void **extra, GucSource source);
+extern void assign_log_suppress_errcodes(const char *newval, void *extra);
extern bool check_log_timezone(char **newval, void **extra, GucSource source);
extern void assign_log_timezone(const char *newval, void *extra);
extern const char *show_log_timezone(void);
--
2.48.1
On Fri, 2025-03-14 at 06:58 +0100, Laurenz Albe wrote:
On Tue, 2025-03-11 at 10:46 +0100, Laurenz Albe wrote:
Attached is the fifth version of the patch.
... and here, for good measure, a pgindented version
Apart from that, no changes.
... and here is v7, improved in the spirit of
/messages/by-id/E132D362-A669-4606-AFE1-B45C9DFCC141@yesql.se
Yours,
Laurenz Albe
Attachments:
v7-0001-Add-parameter-log_suppress_errcodes.patchtext/x-patch; charset=UTF-8; name=v7-0001-Add-parameter-log_suppress_errcodes.patchDownload
From c02f26c9038f0142c202fc8b5522bb7beb95dfce Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Sat, 15 Mar 2025 07:09:15 +0100
Subject: [PATCH v7] Add parameter log_suppress_errcodes
The parameter contains a list of SQLSTATEs for which
FATAL and ERROR messages are not logged. This is to
suppress messages that are of little interest to the
database administrator, but tend to clutter the log.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Discussion: https://postgr.es/m/408f399e7de1416c47bab7e260327ed5ad92838c.camel%40cybertec.at
---
doc/src/sgml/config.sgml | 35 ++++
src/backend/utils/error/elog.c | 156 ++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 11 ++
src/backend/utils/misc/postgresql.conf.sample | 3 +
src/include/pg_config_manual.h | 10 ++
src/include/utils/guc.h | 1 +
src/include/utils/guc_hooks.h | 2 +
7 files changed, 218 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d2fa5f7d1a9..ba5f60977a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6916,6 +6916,41 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
+ <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
+ from client backend processes with certain error codes to be excluded
+ from the log.
+ The value is a comma-separated list of five-character error codes as
+ listed in <xref linkend="errcodes-appendix"/>. An error code that
+ represents a class of errors (ends with three zeros) suppresses logging
+ of all error codes within that class. For example, the entry
+ <literal>08000</literal> (<literal>connection_exception</literal>)
+ would suppress an error with code <literal>08P01</literal>
+ (<literal>protocol_violation</literal>). The default setting is
+ empty, that is, all error codes get logged.
+ Only superusers and users with the appropriate <literal>SET</literal>
+ privilege can change this setting.
+ </para>
+
+ <para>
+ This setting allows you to skip error logging for messages that are
+ frequent but irrelevant. Supressing such messages makes it easier to
+ spot relevant messages in the log and keeps log files from growing too
+ big. For example, if you have a monitoring system that regularly
+ establishes a TCP connection to the server without sending a correct
+ startup packet, you could suppress the protocol violation errors by
+ adding the error code <literal>08P01</literal> to the list.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-min-duration-statement" xreflabel="log_min_duration_statement">
<term><varname>log_min_duration_statement</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42..d88bbc757b4 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -109,12 +109,16 @@ int Log_error_verbosity = PGERROR_DEFAULT;
char *Log_line_prefix = NULL; /* format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+char *log_suppress_errcodes = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
/* Processed form of backtrace_functions GUC */
static char *backtrace_function_list;
+/* Processed form form of log_suppress_errcodes (zero-terminated array) */
+static int *suppressed_errcodes;
+
#ifdef HAVE_SYSLOG
/*
@@ -859,6 +863,30 @@ errcode(int sqlerrcode)
edata->sqlerrcode = sqlerrcode;
+ /*
+ * ERROR and FATAL messages with codes in log_suppress_errcodes don't get
+ * logged. We only want to suppress error messages from ordinary backend
+ * processes.
+ */
+ if ((MyBackendType == B_BACKEND ||
+ MyBackendType == B_STANDALONE_BACKEND) &&
+ (edata->elevel == ERROR ||
+ edata->elevel == FATAL) &&
+ suppressed_errcodes != NULL)
+ {
+ int *state;
+
+ for (state = suppressed_errcodes; *state != 0; state++)
+ /* error categories match all error codes in the category */
+ if (sqlerrcode == *state ||
+ (ERRCODE_IS_CATEGORY(*state) &&
+ ERRCODE_TO_CATEGORY(sqlerrcode) == *state))
+ {
+ edata->output_to_server = false;
+ break;
+ }
+ }
+
return 0; /* return value does not matter */
}
@@ -2299,6 +2327,134 @@ assign_log_destination(const char *newval, void *extra)
Log_destination = *((int *) extra);
}
+/*
+ * GUC check_hook for log_suppress_errcodes
+ *
+ * Split the string on the commas, check the SQLSTATEs for validity, convert
+ * them into the packed integer form and store them in a 0-terminated array.
+ * That array is returned as "extra" and will be assigned to
+ * "suppressed_errcodes" in the assign hook.
+ *
+ * Replace the actual parameter with a sanitized version reassembled from that
+ * array.
+ */
+bool
+check_log_suppress_errcodes(char **newval, void **extra, GucSource source)
+{
+ /* SplitIdentifierString modifies the string */
+ char *new_copy = pstrdup(*newval);
+ int listlen;
+ int *statelist = NULL;
+ int index = 0;
+ int param_len = 1; /* size of the parameter value replacement */
+ List *states;
+ ListCell *c;
+
+ if (!SplitIdentifierString(new_copy, ',', &states))
+ {
+ GUC_check_errdetail("List syntax is invalid.");
+ goto failed;
+ }
+
+ listlen = list_length(states);
+ /* we need guc_malloc(), because that will be returned in "extra" */
+ statelist = guc_malloc(LOG, sizeof(int) * (listlen + 1));
+ if (!statelist)
+ goto failed;
+
+ /* check all error codes for validity and compile them into statelist */
+ foreach(c, states)
+ {
+ char *state = lfirst(c);
+ char *p;
+ int errcode;
+ int i;
+ bool duplicate = false;
+
+ if (strlen(state) != 5)
+ {
+ GUC_check_errdetail("error codes must have 5 characters.");
+ goto failed;
+ }
+
+ /*
+ * Check the the values are alphanumeric and convert them to upper
+ * case (SplitIdentifierString converted them to lower case).
+ */
+ for (p = state; *p != '\0'; p++)
+ if (*p >= 'a' && *p <= 'z')
+ *p += 'A' - 'a';
+ else if (*p < '0' || *p > '9')
+ {
+ GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
+ goto failed;
+ }
+
+ errcode = MAKE_SQLSTATE(state[0], state[1], state[2], state[3], state[4]);
+ /* ignore 0: it cannot be an error code, and we use it to end the list */
+ if (errcode == ERRCODE_SUCCESSFUL_COMPLETION)
+ continue;
+
+ /* ignore duplicate entries */
+ for (i = 0; i < index; i++)
+ if (statelist[i] == errcode)
+ duplicate = true;
+ if (duplicate)
+ continue;
+
+ statelist[index++] = errcode;
+ param_len += 6;
+ }
+ statelist[index] = 0;
+
+ /* will be assigned to "suppressed_errcodes" */
+ *extra = statelist;
+
+ /*
+ * Now that we have successfully parsed the SQLSTATEs, reassemble a string
+ * list for the actual parameter value. That will remove extra spaces and
+ * duplicates and convert the values to upper case.
+ */
+ guc_free(*newval);
+ *newval = guc_malloc(LOG, param_len);
+ if (!*newval)
+ goto failed;
+
+ list_free(states);
+ pfree(new_copy);
+
+ (*newval)[0] = '\0';
+ index = 0;
+ while (statelist[index] != 0)
+ {
+ if (index > 0)
+ strncat(*newval, ",", 2);
+ strncat(*newval, unpack_sql_state(statelist[index]), 6);
+ index++;
+ }
+
+ return true;
+
+failed:
+ list_free(states);
+ pfree(new_copy);
+ guc_free(statelist); /* won't gag on NULL */
+ return false;
+}
+
+/*
+ * GUC assign_hook for log_suppress_errcodes
+ */
+void
+assign_log_suppress_errcodes(const char *newval, void *extra)
+{
+ /* store NULL instead of an empty array for performance */
+ if (*(int *) extra == 0)
+ suppressed_errcodes = NULL;
+ else
+ suppressed_errcodes = extra;
+}
+
/*
* GUC assign_hook for syslog_ident
*/
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..02f0570ef2f 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4635,6 +4635,17 @@ struct config_string ConfigureNamesString[] =
check_canonical_path, NULL, NULL
},
+ {
+ {"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
+ NULL,
+ GUC_LIST_INPUT
+ },
+ &log_suppress_errcodes,
+ DEFAULT_LOG_SUPPRESS_ERRCODES,
+ check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+ },
+
{
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2d1de9c37bd..24fa9163fee 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -541,6 +541,9 @@
# fatal
# panic (effectively off)
+#log_suppress_errcodes = '' # FATAL and ERROR messages with these error codes
+ # are not logged
+
#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements
# and their durations, > 0 logs only
# statements running at least this number
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 449e50bd78c..7fc90ea18c4 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -200,6 +200,16 @@
*/
#define DEFAULT_EVENT_SOURCE "PostgreSQL"
+/*
+ * Default value for log_suppress_errcodes. ERROR or FATAL messages with
+ * these error codes are never logged. Error classes (error codes ending with
+ * three zeros) match all error codes in the class. The idea is to suppress
+ * messages that usually don't indicate a serious problem but tend to pollute
+ * the log file.
+ */
+
+#define DEFAULT_LOG_SUPPRESS_ERRCODES ""
+
/*
* Assumed cache line size. This doesn't affect correctness, but can be used
* for low-level optimizations. This is mostly used to pad various data
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1233e07d7da..4f29133ae7c 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -271,6 +271,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
+extern PGDLLIMPORT char *log_suppress_errcodes;
extern PGDLLIMPORT int log_min_messages;
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 951451a9765..e55803a0fea 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -76,6 +76,8 @@ extern bool check_log_destination(char **newval, void **extra,
extern void assign_log_destination(const char *newval, void *extra);
extern const char *show_log_file_mode(void);
extern bool check_log_stats(bool *newval, void **extra, GucSource source);
+extern bool check_log_suppress_errcodes(char **newval, void **extra, GucSource source);
+extern void assign_log_suppress_errcodes(const char *newval, void *extra);
extern bool check_log_timezone(char **newval, void **extra, GucSource source);
extern void assign_log_timezone(const char *newval, void *extra);
extern const char *show_log_timezone(void);
--
2.48.1
On 15.03.25 07:12, Laurenz Albe wrote:
... and here is v7, improved in the spirit of
/messages/by-id/E132D362-A669-4606-AFE1-B45C9DFCC141@yesql.se
I just revisited this patch and v7 passes all tests from [1,2].
LGTM.
Best regards, Jim
[1]: /messages/by-id/205505aa-4ba1-4d5b-b5f2-d006f14f4775@uni-muenster.de
/messages/by-id/205505aa-4ba1-4d5b-b5f2-d006f14f4775@uni-muenster.de
[2]: /messages/by-id/ec933afb-c8a3-4954-bb58-8f86b272a6f9@uni-muenster.de
/messages/by-id/ec933afb-c8a3-4954-bb58-8f86b272a6f9@uni-muenster.de
On 2025/03/17 17:12, Jim Jones wrote:
On 15.03.25 07:12, Laurenz Albe wrote:
... and here is v7, improved in the spirit of
/messages/by-id/E132D362-A669-4606-AFE1-B45C9DFCC141@yesql.seI just revisited this patch and v7 passes all tests from [1,2].
LGTM.
Since this patch is marked as ready for committer, I've started looking at it.
Filtering log messages by SQLSTATE might be useful for some users,
but I'm unsure if it should belong in the core. There are also other
potential filtering needs, such as by application name, client host,
database, or roles. Adding only SQLSTATE filtering may not be good idea,
while supporting all possible cases in the core wouldn't be practical either.
Given that, I think implementing this as an extension using emit_log_hook
would be a better approach. Anyway, I'd like to hear more opinions from
other hackers on this.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
Filtering log messages by SQLSTATE might be useful for some users,
but I'm unsure if it should belong in the core. There are also other
potential filtering needs, such as by application name, client host,
database, or roles. Adding only SQLSTATE filtering may not be good idea,
while supporting all possible cases in the core wouldn't be practical either.
Given that, I think implementing this as an extension using emit_log_hook
would be a better approach. Anyway, I'd like to hear more opinions from
other hackers on this.
I took a brief look and I concur with Fujii-san's conclusion: this'd
be a fine extension a/k/a contrib module, but it seems a bit too
opinionated about what sort of filtering is needed to be appropriate
within elog.c itself.
Also, just as a minor coding thought, I'd suggest using -1 not 0
to terminate the array of encoded SQLSTATEs, rather than assuming
that nobody would write 00000. Compare commit 58fdca220.
regards, tom lane
On Wed, 2025-04-02 at 15:23 -0400, Tom Lane wrote:
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
Filtering log messages by SQLSTATE might be useful for some users,
but I'm unsure if it should belong in the core. There are also other
potential filtering needs, such as by application name, client host,
database, or roles. Adding only SQLSTATE filtering may not be good idea,
while supporting all possible cases in the core wouldn't be practical either.Given that, I think implementing this as an extension using emit_log_hook
would be a better approach. Anyway, I'd like to hear more opinions from
other hackers on this.I took a brief look and I concur with Fujii-san's conclusion: this'd
be a fine extension a/k/a contrib module, but it seems a bit too
opinionated about what sort of filtering is needed to be appropriate
within elog.c itself.Also, just as a minor coding thought, I'd suggest using -1 not 0
to terminate the array of encoded SQLSTATEs, rather than assuming
that nobody would write 00000. Compare commit 58fdca220.
I agree with these assessments and will withdraw the patch.
Yours,
Laurenz Albe