Re: Support a wildcard in backtrace_functions

From: Jelte Fennema-Nio <me(at)jeltef(dot)nl>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Support a wildcard in backtrace_functions
Date: 2024-03-11 09:52:59
Message-ID: CAGECzQTVou9VZWNq9DqHLET1A7jHf7PXzUiC_PA2v1MH_B1uEg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 8 Mar 2024 at 17:17, Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> Hm, we may not want backtrace_on_internal_error to be on by default.
> AFAICS, all ERRCODE_INTERNAL_ERROR are identifiable with the error
> message, so it's sort of easy for one to track down the cause of it
> without actually needing to log backtrace by default.

While maybe all messages uniquely identify the exact error, these
errors usually originate somewhere deep down the call stack in a
function that is called from many other places. Having the full stack
trace can thus greatly help us to find what caused this specific error
to occur. I think that would be quite useful to enable by default, if
only because it would make many bug reports much more actionable.

> 1. Rename the new GUC backtrace_functions_min_level to backtrace_min_level.
> 2. Add 'internal' to backtrace_min_level_options enum
> +static const struct config_enum_entry backtrace_functions_level_options[] = {
> + {"internal", INTERNAL, false},
> + {"debug5", DEBUG5, false},
> + {"debug4", DEBUG4, false},
> 3. Remove backtrace_on_internal_error GUC which is now effectively
> covered by backtrace_min_level = 'internal';
>
> Does anyone see a problem with it?

Honestly, it seems a bit confusing to me to add INTERNAL as a level,
since it's an error code not log level. Also merging it in this way,
brings up certain questions:
1. How do you get the current backtrace_on_internal_error=true
behaviour? Would you need to set both backtrace_functions='*' and
backtrace_min_level=INTERNAL?
2. What is the default value for backtrace_min_level?
3. You still wouldn't be able to limit INTERNAL errors to FATAL level

I personally think having three GUCs in this patchset make sense,
especially since I think it would be good to turn
backtrace_on_internal_error on by default. The only additional change
that I think might be worth making is to make
backtrace_on_internal_error take a level argument, so that you could
configure postgres to only add stack traces to FATAL internal errors.

(attached is a patch that should fix the CI issue by adding
GUC_NOT_IN_SAMPLE backtrace_functions_min_level)

Attachment Content-Type Size
v7-0002-Add-wildcard-support-to-backtrace_functions-GUC.patch application/octet-stream 2.1 KB
v7-0001-Add-backtrace_functions_min_level.patch application/octet-stream 6.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2024-03-11 10:00:32 Re: Statistics Import and Export
Previous Message Alexander Korotkov 2024-03-11 09:48:25 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.