On Fri, Apr 12, 2024 at 09:36:36AM +0900, Michael Paquier wrote: > log_backtrace speaks a bit more to me as a name for this stuff because > it logs a backtrace. Now, there is consistency on HEAD as well > because these GUCs are all prefixed with "backtrace_". Would > something like a backtrace_mode where we have an enum rather than a > boolean be better? One thing would be to redesign the existing GUC as > having two values on HEAD as of: > - "none", to log nothing. > - "internal", to log backtraces for internal errors. > > Then this could be extended with more modes, to discuss in future > releases as new features.
As this is an open item, let's move on here. Attached is a proposal of patch for this open item, switching backtrace_on_internal_error to backtrace_mode with two values: - "none", to log no backtraces. - "internal", to log backtraces for internal errors. The rest of the proposals had better happen as a v18 discussion, where extending this GUC is a benefit. -- Michael
From 348c3d45ddf4c29163c6963cb640c372fdbe72d5 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 18 Apr 2024 15:57:10 +0900
Subject: [PATCH] backtrace_on_internal_error -> backtrace_mode
This currently supports two values:
- "none", to log no backtraces.
- "internal", to log a backtrace with an internal error.
---
src/include/utils/elog.h | 6 ++++++
src/include/utils/guc.h | 2 +-
src/backend/utils/error/elog.c | 2 +-
src/backend/utils/misc/guc_tables.c | 33 +++++++++++++++++++----------
doc/src/sgml/config.sgml | 19 ++++++++++-------
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 42 insertions(+), 21 deletions(-)
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 054dd2bf62..b132f89e98 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -495,6 +495,12 @@ typedef enum
PGERROR_VERBOSE, /* all the facts, ma'am */
} PGErrorVerbosity;
+typedef enum
+{
+ BACKTRACE_MODE_NONE, /* no backtraces */
+ BACKTRACE_MODE_INTERNAL, /* backtraces for internal error code */
+} BacktraceMode;
+
extern PGDLLIMPORT int Log_error_verbosity;
extern PGDLLIMPORT char *Log_line_prefix;
extern PGDLLIMPORT int Log_destination;
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 8d1fe04078..53dd28222a 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -267,7 +267,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 backtrace_mode;
extern PGDLLIMPORT int temp_file_limit;
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 605ff3b045..1c108b86ec 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -501,7 +501,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
backtrace_functions &&
matches_backtrace_functions(edata->funcname)) ||
(edata->sqlerrcode == ERRCODE_INTERNAL_ERROR &&
- backtrace_on_internal_error)))
+ backtrace_mode == BACKTRACE_MODE_INTERNAL)))
set_backtrace(edata, 2);
/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c68fdc008b..d5a9f3b73e 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -79,6 +79,7 @@
#include "tsearch/ts_cache.h"
#include "utils/builtins.h"
#include "utils/bytea.h"
+#include "utils/elog.h"
#include "utils/float.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
@@ -117,6 +118,15 @@ extern bool optimize_bounded_sort;
* NOTE! Option values may not contain double quotes!
*/
+static const struct config_enum_entry backtrace_mode_options[] = {
+ {"none", BACKTRACE_MODE_NONE, false},
+ {"internal", BACKTRACE_MODE_INTERNAL, false},
+ {NULL, 0, false}
+};
+
+StaticAssertDecl(lengthof(backtrace_mode_options) == (BACKTRACE_MODE_INTERNAL + 2),
+ "array length mismatch");
+
static const struct config_enum_entry bytea_output_options[] = {
{"escape", BYTEA_OUTPUT_ESCAPE, false},
{"hex", BYTEA_OUTPUT_HEX, false},
@@ -531,7 +541,7 @@ int log_temp_files = -1;
double log_statement_sample_rate = 1.0;
double log_xact_sample_rate = 0;
char *backtrace_functions;
-bool backtrace_on_internal_error = false;
+int backtrace_mode = BACKTRACE_MODE_NONE;
int temp_file_limit = -1;
@@ -770,16 +780,6 @@ 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,
- false,
- NULL, NULL, NULL
- },
{
{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of sequential-scan plans."),
@@ -4745,6 +4745,17 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"backtrace_mode", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Controls backtrace logging."),
+ NULL,
+ GUC_NOT_IN_SAMPLE
+ },
+ &backtrace_mode,
+ BACKTRACE_MODE_NONE, backtrace_mode_options,
+ NULL, NULL, NULL
+ },
+
{
{"bytea_output", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the output format for bytea."),
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d8e1282e12..b9a317b15d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -11381,19 +11381,22 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
- <varlistentry id="guc-backtrace-on-internal-error" xreflabel="backtrace_on_internal_error">
- <term><varname>backtrace_on_internal_error</varname> (<type>boolean</type>)
+ <varlistentry id="guc-backtrace-mode" xreflabel="backtrace_mode">
+ <term><varname>backtrace_mode</varname> (<type>enum</type>)
<indexterm>
- <primary><varname>backtrace_on_internal_error</varname> configuration parameter</primary>
+ <primary><varname>backtrace_mode</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.
+ Controls how backtraces should be logged.
+ If this parameter is <literal>internal</literal> 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
+ <literal>none</literal>, to not log backtraces.
</para>
<para>
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d551ada325..6119a5afb3 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -240,6 +240,7 @@ BackgroundWorker
BackgroundWorkerArray
BackgroundWorkerHandle
BackgroundWorkerSlot
+BacktraceMode
BackupState
Barrier
BaseBackupCmd
--
2.43.0
signature.asc
Description: PGP signature
