Feedback-ID: ie0a040ee:Fastmail
Message-ID: <93f32d1a-9309-48dc-9b7f-c110c4d48126@eisentraut.org>
Date: Thu, 7 Dec 2023 11:00:11 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: backtrace_on_internal_error
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
References: <ba76c6bc-f03f-4285-bf16-47759cfcab9e@eisentraut.org>
In-Reply-To: <ba76c6bc-f03f-4285-bf16-47759cfcab9e@eisentraut.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/93f32d1a-9309-48dc-9b7f-c110c4d48126%40eisentraut.org>
Precedence: bulk

On 05.12.23 11:55, Peter Eisentraut wrote:
> I think the implementation would be very simple, something like
> 
> diff --git a/src/backend/utils/error/elog.c 
> b/src/backend/utils/error/elog.c
> index 6aeb855e49..45d40abe92 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);
> 
>      /*
> 
> where backtrace_on_internal_error would be a GUC variable.

It looks like many people found this idea worthwhile.

Several people asked for a way to set the minimum log level for this 
treatment.

Something else to note:  I wrote the above code to check the error code; 
it doesn't check whether the original code write elog() or ereport(). 
There are some internal errors that are written as ereport() now. 
Others might be changed from time to time; until now there would have 
been no external effect from this.  I think it would be weird to 
introduce a difference between these forms now.

But then, elog() only uses the error code ERRCODE_INTERNAL_ERROR if the 
error level is >=ERROR.  So this already excludes everything below.

Do people want a way to distinguish ERROR/FATAL/PANIC?

Or do people want a way to enable backtraces for elog(LOG)?  This didn't 
look too interesting to me.  (Many current elog(LOG) calls are behind 
additional guards like TRACE_SORT or LOCK_DEBUG.)

If neither of these two are very interesting, then the above code would 
already appear to do what was asked.