On Thu, Mar 21, 2024 at 8:11 PM Jelte Fennema-Nio <m...@jeltef.nl> wrote:
>
> Attached is a patch that implements this. Since the more I think about
> it, the more I like this approach.
Thanks. Overall the design looks good. log_backtrace is the entry
point for one to control if a backtrace needs to be emitted at all.
backtrace_min_level controls at what elevel the backtraces need to be
emitted.
If one wants to get backtraces for all internal ERRORs, then
log_backtrace = 'internal' and backtrace_min_level = 'error' must be
set. If backtrace_min_level = 'panic', then backtraces won't get
logged for internal ERRORs. But, this is not the case right now, one
can just set backtrace_on_internal_error = 'on' to get backtraces for
all internal ERROR/FATAL or whatever just the errcode has to be
ERRCODE_INTERNAL_ERROR. This is one change of behaviour and looks fine
to me.
If one wants to get backtrace from a function for all elog/ereport
calls, then log_backtrace = either 'internal' or 'all',
backtrace_min_level = 'debug5' and backtrace_functions =
'<function_name>' must be set. But, right now, one can just set
backtrace_functions = '<function_name>' to get backtrace from the
functions for any of elog/ereport calls.
A few comments:
1.
@@ -832,6 +849,9 @@ matches_backtrace_functions(const char *funcname)
{
const char *p;
+ if (!backtrace_functions || backtrace_functions[0] == '\0')
+ return true;
+
Shouldn't this be returning false to not log set backtrace when
backtrace_functions is not set? Am I missing something here?
2.
+ {
+ {"log_backtrace", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Sets if logs should include a backtrace."),
+ NULL
IMV, log_backtrace, backtrace_min_level and backtrace_functions are
interlinked, so keeping all of them as DEVELOPER_OPTIONS looks fine to
me than having log_backtrace at just LOGGING_WHAT kind. Also, we must
also mark log_backtrace as GUC_NOT_IN_SAMPLE.
3. I think it's worth adding a few examples to get backtraces in docs.
For instance, what to set to get backtraces of all internal errors and
what to set to get backtraces of all ERRORs coming from a specific
function etc.
4. I see the support for wildcard in backtrace_functions is missing.
Is it intentionally left out? If not, can you make it part of 0003
patch?
5. The amount of backtraces generated is going to be too huge when
setting log_backtrace = 'all' and backtrace_min_level = 'debug5'. With
this setting installcheck generated 12MB worth of log and the test
took about 55 seconds (as opposed to 48 seconds without it) The point
is if these settings are misused, it can easily slow down the entire
system and fill up disk space leading to crashes eventually. This
makes a strong case for marking log_backtrace a developer only
function.
6. In continuation to comment #5, does anyone need backtrace for
elevels like debugX and LOG etc.? What's the use of the backtrace in
such cases?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
