public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re: backtrace_on_internal_error
Date: Fri, 8 Dec 2023 14:21:32 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>

Here is a patch to play with.

I also found a related typo.

One possible question for discussion is whether the default for this 
should be off, on, or possibly something like on-in-assert-builds. 
(Personally, I'm happy to turn it on myself at run time, but everyone 
has different workflows.)


From 2e01ee20358df323170fbfef6dc01f684371d873 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 8 Dec 2023 13:30:55 +0100
Subject: [PATCH v1 1/2] Fix variable name and comment

---
 src/backend/utils/error/elog.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 6aeb855e49..ecacba4ee1 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -115,8 +115,8 @@ char	   *Log_destination_string = NULL;
 bool		syslog_sequence_numbers = true;
 bool		syslog_split_messages = true;
 
-/* Processed form of backtrace_symbols GUC */
-static char *backtrace_symbol_list;
+/* Processed form of backtrace_functions GUC */
+static char *backtrace_function_list;
 
 #ifdef HAVE_SYSLOG
 
@@ -831,13 +831,13 @@ matches_backtrace_functions(const char *funcname)
 {
 	const char *p;
 
-	if (!backtrace_symbol_list || funcname == NULL || funcname[0] == '\0')
+	if (!backtrace_function_list || funcname == NULL || funcname[0] == '\0')
 		return false;
 
-	p = backtrace_symbol_list;
+	p = backtrace_function_list;
 	for (;;)
 	{
-		if (*p == '\0')			/* end of backtrace_symbol_list */
+		if (*p == '\0')			/* end of backtrace_function_list */
 			break;
 
 		if (strcmp(funcname, p) == 0)
@@ -2180,7 +2180,7 @@ check_backtrace_functions(char **newval, void **extra, GucSource source)
 void
 assign_backtrace_functions(const char *newval, void *extra)
 {
-	backtrace_symbol_list = (char *) extra;
+	backtrace_function_list = (char *) extra;
 }
 
 /*

base-commit: 7db01fbcefbd95a7c97afa128fab59f4a09b3ff1
-- 
2.43.0


From 06beed94067efaf5b93da5041048347fa22a4290 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 8 Dec 2023 14:04:53 +0100
Subject: [PATCH v1 2/2] Add GUC backtrace_on_internal_error

---
 doc/src/sgml/config.sgml            | 27 +++++++++++++++++++++++++++
 src/backend/utils/error/elog.c      |  8 +++++---
 src/backend/utils/misc/guc_tables.c | 16 ++++++++++++++++
 src/include/utils/guc.h             |  1 +
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 94d1eb2b81..b830539b9a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11027,6 +11027,33 @@ <title>Developer Options</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-backtrace-on-internal-error" xreflabel="backtrace_on_internal_error">
+      <term><varname>backtrace_on_internal_error</varname> (<type>boolean</type>)
+      <indexterm>
+        <primary><varname>backtrace_on_internal_error</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        If this parameter is on and an error with error code XX000 (internal
+        error; see also <xref linkend="errcodes-appendix"/>) is raised, then a
+        backtrace is written to the server log together with the error
+        message.  This can be used to debug such internal errors (which should
+        normally not happen in production).  The default is off.
+       </para>
+
+       <para>
+        Backtrace support is not available on all platforms, and the quality
+        of the backtraces depends on compilation options.
+       </para>
+
+       <para>
+        Only superusers and users with the appropriate <literal>SET</literal>
+        privilege can change this setting.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-debug-discard-caches" xreflabel="debug_discard_caches">
       <term><varname>debug_discard_caches</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index ecacba4ee1..6a7a487323 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -498,9 +498,11 @@ errfinish(const char *filename, int lineno, const char *funcname)
 
 	/* Collect backtrace, if enabled and we didn't already */
 	if (!edata->backtrace &&
-		edata->funcname &&
-		backtrace_functions &&
-		matches_backtrace_functions(edata->funcname))
+		((edata->funcname &&
+		  backtrace_functions &&
+		  matches_backtrace_functions(edata->funcname)) ||
+		 (edata->sqlerrcode == ERRCODE_INTERNAL_ERROR &&
+		  backtrace_on_internal_error)))
 		set_backtrace(edata, 2);
 
 	/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 6474e35ec0..73252d4864 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -527,6 +527,7 @@ double		log_statement_sample_rate = 1.0;
 double		log_xact_sample_rate = 0;
 int			trace_recovery_messages = LOG;
 char	   *backtrace_functions;
+bool		backtrace_on_internal_error = false;
 
 int			temp_file_limit = -1;
 
@@ -810,6 +811,21 @@ StaticAssertDecl(lengthof(config_type_names) == (PGC_ENUM + 1),
 
 struct config_bool ConfigureNamesBool[] =
 {
+	{
+		{"backtrace_on_internal_error", PGC_SUSET, DEVELOPER_OPTIONS,
+			gettext_noop("Log backtrace for any error with error code XX000 (internal error)."),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&backtrace_on_internal_error,
+
+		/*
+		 * TODO: Discussion: Could default to true unconditionally or in
+		 * assert builds?
+		 */
+		false,
+		NULL, NULL, NULL
+	},
 	{
 		{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
 			gettext_noop("Enables the planner's use of sequential-scan plans."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 20fe13702b..b4df0b6678 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -264,6 +264,7 @@ extern PGDLLIMPORT int log_temp_files;
 extern PGDLLIMPORT double log_statement_sample_rate;
 extern PGDLLIMPORT double log_xact_sample_rate;
 extern PGDLLIMPORT char *backtrace_functions;
+extern PGDLLIMPORT bool backtrace_on_internal_error;
 
 extern PGDLLIMPORT int temp_file_limit;
 
-- 
2.43.0



Attachments:

  [text/plain] v1-0001-Fix-variable-name-and-comment.patch (1.6K, ../[email protected]/2-v1-0001-Fix-variable-name-and-comment.patch)
  download | inline diff:
From 2e01ee20358df323170fbfef6dc01f684371d873 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 8 Dec 2023 13:30:55 +0100
Subject: [PATCH v1 1/2] Fix variable name and comment

---
 src/backend/utils/error/elog.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 6aeb855e49..ecacba4ee1 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -115,8 +115,8 @@ char	   *Log_destination_string = NULL;
 bool		syslog_sequence_numbers = true;
 bool		syslog_split_messages = true;
 
-/* Processed form of backtrace_symbols GUC */
-static char *backtrace_symbol_list;
+/* Processed form of backtrace_functions GUC */
+static char *backtrace_function_list;
 
 #ifdef HAVE_SYSLOG
 
@@ -831,13 +831,13 @@ matches_backtrace_functions(const char *funcname)
 {
 	const char *p;
 
-	if (!backtrace_symbol_list || funcname == NULL || funcname[0] == '\0')
+	if (!backtrace_function_list || funcname == NULL || funcname[0] == '\0')
 		return false;
 
-	p = backtrace_symbol_list;
+	p = backtrace_function_list;
 	for (;;)
 	{
-		if (*p == '\0')			/* end of backtrace_symbol_list */
+		if (*p == '\0')			/* end of backtrace_function_list */
 			break;
 
 		if (strcmp(funcname, p) == 0)
@@ -2180,7 +2180,7 @@ check_backtrace_functions(char **newval, void **extra, GucSource source)
 void
 assign_backtrace_functions(const char *newval, void *extra)
 {
-	backtrace_symbol_list = (char *) extra;
+	backtrace_function_list = (char *) extra;
 }
 
 /*

base-commit: 7db01fbcefbd95a7c97afa128fab59f4a09b3ff1
-- 
2.43.0



  [text/plain] v1-0002-Add-GUC-backtrace_on_internal_error.patch (4.2K, ../[email protected]/3-v1-0002-Add-GUC-backtrace_on_internal_error.patch)
  download | inline diff:
From 06beed94067efaf5b93da5041048347fa22a4290 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 8 Dec 2023 14:04:53 +0100
Subject: [PATCH v1 2/2] Add GUC backtrace_on_internal_error

---
 doc/src/sgml/config.sgml            | 27 +++++++++++++++++++++++++++
 src/backend/utils/error/elog.c      |  8 +++++---
 src/backend/utils/misc/guc_tables.c | 16 ++++++++++++++++
 src/include/utils/guc.h             |  1 +
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 94d1eb2b81..b830539b9a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11027,6 +11027,33 @@ <title>Developer Options</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-backtrace-on-internal-error" xreflabel="backtrace_on_internal_error">
+      <term><varname>backtrace_on_internal_error</varname> (<type>boolean</type>)
+      <indexterm>
+        <primary><varname>backtrace_on_internal_error</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        If this parameter is on and an error with error code XX000 (internal
+        error; see also <xref linkend="errcodes-appendix"/>) is raised, then a
+        backtrace is written to the server log together with the error
+        message.  This can be used to debug such internal errors (which should
+        normally not happen in production).  The default is off.
+       </para>
+
+       <para>
+        Backtrace support is not available on all platforms, and the quality
+        of the backtraces depends on compilation options.
+       </para>
+
+       <para>
+        Only superusers and users with the appropriate <literal>SET</literal>
+        privilege can change this setting.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-debug-discard-caches" xreflabel="debug_discard_caches">
       <term><varname>debug_discard_caches</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index ecacba4ee1..6a7a487323 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -498,9 +498,11 @@ errfinish(const char *filename, int lineno, const char *funcname)
 
 	/* Collect backtrace, if enabled and we didn't already */
 	if (!edata->backtrace &&
-		edata->funcname &&
-		backtrace_functions &&
-		matches_backtrace_functions(edata->funcname))
+		((edata->funcname &&
+		  backtrace_functions &&
+		  matches_backtrace_functions(edata->funcname)) ||
+		 (edata->sqlerrcode == ERRCODE_INTERNAL_ERROR &&
+		  backtrace_on_internal_error)))
 		set_backtrace(edata, 2);
 
 	/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 6474e35ec0..73252d4864 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -527,6 +527,7 @@ double		log_statement_sample_rate = 1.0;
 double		log_xact_sample_rate = 0;
 int			trace_recovery_messages = LOG;
 char	   *backtrace_functions;
+bool		backtrace_on_internal_error = false;
 
 int			temp_file_limit = -1;
 
@@ -810,6 +811,21 @@ StaticAssertDecl(lengthof(config_type_names) == (PGC_ENUM + 1),
 
 struct config_bool ConfigureNamesBool[] =
 {
+	{
+		{"backtrace_on_internal_error", PGC_SUSET, DEVELOPER_OPTIONS,
+			gettext_noop("Log backtrace for any error with error code XX000 (internal error)."),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&backtrace_on_internal_error,
+
+		/*
+		 * TODO: Discussion: Could default to true unconditionally or in
+		 * assert builds?
+		 */
+		false,
+		NULL, NULL, NULL
+	},
 	{
 		{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
 			gettext_noop("Enables the planner's use of sequential-scan plans."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 20fe13702b..b4df0b6678 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -264,6 +264,7 @@ extern PGDLLIMPORT int log_temp_files;
 extern PGDLLIMPORT double log_statement_sample_rate;
 extern PGDLLIMPORT double log_xact_sample_rate;
 extern PGDLLIMPORT char *backtrace_functions;
+extern PGDLLIMPORT bool backtrace_on_internal_error;
 
 extern PGDLLIMPORT int temp_file_limit;
 
-- 
2.43.0



view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: backtrace_on_internal_error
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox