Hi,
This patch is currently parked in the July CommitFest:
https://commitfest.postgresql.org/48/4735/
That's fine, except that I think that what the previous discussion
revealed is that we don't have consensus on how backtrace behavior
ought to be controlled: backtrace_on_internal_error was one
On Mon, Apr 29, 2024 at 08:08:19AM -0400, Robert Haas wrote:
> On Mon, Apr 29, 2024 at 5:12 AM Peter Eisentraut wrote:
>> Ok, it's reverted.
Thanks for taking care of it.
> Thanks, and sorry. :-(
Sorry for the outcome..
--
Michael
signature.asc
Description: PGP signature
On Mon, Apr 29, 2024 at 5:12 AM Peter Eisentraut wrote:
> On 27.04.24 00:16, Michael Paquier wrote:
> > On Fri, Apr 26, 2024 at 02:39:16PM -0400, Tom Lane wrote:
> >> Well, in that case we have to have some kind of control GUC, and
> >> I think the consensus is that the one we have now is
On 27.04.24 00:16, Michael Paquier wrote:
On Fri, Apr 26, 2024 at 02:39:16PM -0400, Tom Lane wrote:
Well, in that case we have to have some kind of control GUC, and
I think the consensus is that the one we have now is under-designed.
So I also vote for a full revert and try again in v18.
On Fri, Apr 26, 2024 at 02:39:16PM -0400, Tom Lane wrote:
> Well, in that case we have to have some kind of control GUC, and
> I think the consensus is that the one we have now is under-designed.
> So I also vote for a full revert and try again in v18.
Okay, fine by me to move on with a revert.
On 2024-04-26 14:39:16 -0400, Tom Lane wrote:
> Andres Freund writes:
> > I don't think enabling backtraces without a way to disable them is a good
> > idea
> > - security vulnerablilities in backtrace generation code are far from
> > unheard
> > of and can make error handling a lot slower...
>
Andres Freund writes:
> I don't think enabling backtraces without a way to disable them is a good idea
> - security vulnerablilities in backtrace generation code are far from unheard
> of and can make error handling a lot slower...
Well, in that case we have to have some kind of control GUC, and
Hi,
On 2024-04-19 15:24:17 -0400, Tom Lane wrote:
> I can't say that I care for "backtrace_on_internal_error".
> Re-reading that thread, I see I argued for having *no* GUC and
> just enabling that behavior all the time. I lost that fight,
> but it should have been clear that a GUC of this exact
On Tue, Apr 23, 2024 at 1:05 AM Michael Paquier wrote:
> At this stage, my opinion would tend in favor of a revert of the GUC.
> The second class of cases is useful to stress many unexpected cases,
> and I don't expect this number to go down over time, but increase with
> more advanced tests
On Mon, Apr 22, 2024 at 09:25:15AM -0400, Robert Haas wrote:
> That's long been my feeling about this. So, if we revert this for now,
> what we ought to do is put it back right ASAP after feature freeze and
> then clean all that up.
In the 85 backtraces I can find in the tests, we have a mixed
On Mon, Apr 22, 2024 at 2:36 AM Michael Paquier wrote:
> I'd like to think about this stuff in a different way: this is useful
> if enabled by default because it can also help in finding out error
> paths that should not use the internal errcode. Normally, there
> should be zero backtraces
On Sun, Apr 21, 2024 at 09:26:38AM +0200, Peter Eisentraut wrote:
> Note that a standard test run produces a number of internal errors. I
> haven't checked how likely these are in production, but one might want to
> consider that before starting to dump backtraces in routine situations.
>
> For
On Sat, 20 Apr 2024 at 01:19, Michael Paquier wrote:
> Removing this GUC and making the backend react by default the same way
> as when this GUC was enabled sounds like a sensible route here. This
> stuff is useful.
I definitely agree it's useful. But I feel like changing the default
of the GUC
On 19.04.24 21:24, Tom Lane wrote:
But on the other hand, in my personal experience,
backtrace_on_internal_error would be the right thing in a really large
number of cases.
That's why I thought we could get away with doing it automatically.
Sure, more control would be better. But if we just
On Fri, Apr 19, 2024 at 04:17:18PM -0400, Robert Haas wrote:
> On Fri, Apr 19, 2024 at 3:24 PM Tom Lane wrote:
>> I can't say that I care for "backtrace_on_internal_error".
>> Re-reading that thread, I see I argued for having *no* GUC and
>> just enabling that behavior all the time. I lost that
On Fri, Apr 19, 2024 at 3:24 PM Tom Lane wrote:
> I can't say that I care for "backtrace_on_internal_error".
> Re-reading that thread, I see I argued for having *no* GUC and
> just enabling that behavior all the time. I lost that fight,
> but it should have been clear that a GUC of this exact
Robert Haas writes:
> On Fri, Apr 19, 2024 at 2:08 PM Tom Lane wrote:
>> I've not been following this thread, so I don't have an opinion
>> about what the design ought to be. But if we still aren't settled
>> on it by now, I think the prudent thing is to revert the feature
>> out of v17
On Fri, Apr 19, 2024 at 2:08 PM Tom Lane wrote:
> Robert Haas writes:
> > There are some things that are pretty hard to change once we've
> > released them. For example, if we had a function or operator and
> > somebody embeds it in a view definition, removing or renaming it
> > prevents people
Robert Haas writes:
> There are some things that are pretty hard to change once we've
> released them. For example, if we had a function or operator and
> somebody embeds it in a view definition, removing or renaming it
> prevents people from upgrading. But GUCs are not as bad.
Really? Are we
On Fri, Apr 19, 2024 at 7:31 AM Jelte Fennema-Nio wrote:
> While there maybe isn't consensus on what a new design exactly looks
> like, I do feel like there's consensus on this thread that the
> backtrace_on_internal_error GUC is almost certainly not the design
> that we want. I guess a more
On Thu, Apr 18, 2024 at 3:02 AM Michael Paquier wrote:
> 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
On Fri, 19 Apr 2024 at 10:58, Peter Eisentraut wrote:
> This presupposes that there is consensus about how the future
> functionality should look like. This topic has gone through half a
> dozen designs over a few months, and I think it would be premature to
> randomly freeze that discussion now
On 18.04.24 11:54, Jelte Fennema-Nio wrote:
On Thu, 18 Apr 2024 at 10:50, Peter Eisentraut wrote:
Why exactly is this an open item? Is there anything wrong with the
existing feature?
The name of the GUC backtrace_on_internal_error is so specific that
it's impossible to extend our backtrace
On Thu, Apr 18, 2024 at 12:21:56PM +0200, Jelte Fennema-Nio wrote:
> On Thu, 18 Apr 2024 at 09:02, Michael Paquier wrote:
>> 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,
On Thu, 18 Apr 2024 at 09:02, Michael Paquier wrote:
>
> 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
On Thu, 18 Apr 2024 at 10:50, Peter Eisentraut wrote:
> Why exactly is this an open item? Is there anything wrong with the
> existing feature?
The name of the GUC backtrace_on_internal_error is so specific that
it's impossible to extend our backtrace behaviour in future releases
without adding
On 18.04.24 09:02, Michael Paquier wrote:
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
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
On Wed, Apr 10, 2024 at 11:07:00AM +0200, Jelte Fennema-Nio wrote:
> I think patch 0002 should be considered an Open Item for PG17. Since
> it's proposing changing the name of the newly introduced
> backtrace_on_internal_error GUC. If we want to change it in this way,
> we should do it before the
On Fri, 22 Mar 2024 at 11:09, Jelte Fennema-Nio wrote:
> On Thu, 21 Mar 2024 at 15:41, Jelte Fennema-Nio wrote:
> > Attached is a patch that implements this. Since the more I think about
> > it, the more I like this approach.
>
> I now added a 3rd patch to this patchset which changes the
>
in 0003)
backtrace_functions = '' (default)
backtrace_min_level = 'ERROR' (default)
and
log_backtrace = 'all'
backtrace_functions = 'someFunc'
backtrace_min_level = 'ERROR' (default)
> 4. I see the support for wildcard in backtrace_functions is missing.
> Is it intentionally left out?
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
On Thu, 21 Mar 2024 at 15:41, Jelte Fennema-Nio wrote:
> Attached is a patch that implements this. Since the more I think about
> it, the more I like this approach.
I now added a 3rd patch to this patchset which changes the
log_backtrace default to "internal", because it seems quite useful to
me
On Wed, 13 Mar 2024 at 16:32, Jelte Fennema-Nio wrote:
> How
> about the following aproach. It still uses 3 GUCs, but they now all
> work together. There's one entry point and two additional filters
> (level and function name)
>
> # Says what log entries to log backtraces for
> log_backtrace =
On Wed, 13 Mar 2024 at 15:20, Peter Eisentraut wrote:
> Hence the idea
>
> backtrace_on_error = {all|internal|none}
>
> which could default to 'internal'.
I think one use-case that I'd personally at least would like to see
covered is being able to get backtraces on all warnings. How would
On Wed, Mar 13, 2024 at 7:50 PM Peter Eisentraut wrote:
>
> > I think it all depends on how close we consider
> > backtrace_on_internal_error and backtrace_functions. While they
> > obviously have similar functionality, I feel like
> > backtrace_on_internal_error is probably a function that we'd
On 08.03.24 16:55, Jelte Fennema-Nio wrote:
On Fri, 8 Mar 2024 at 15:51, Peter Eisentraut wrote:
What is the relationship of these changes with the recently added
backtrace_on_internal_error?
I think that's a reasonable question. And the follow up ones too.
I think it all depends on how
nal errors.
(attached is a patch that should fix the CI issue by adding
GUC_NOT_IN_SAMPLE backtrace_functions_min_level)
v7-0002-Add-wildcard-support-to-backtrace_functions-GUC.patch
Description: Binary data
v7-0001-Add-backtrace_functions_min_level.patch
Description: Binary data
On Fri, Mar 8, 2024 at 9:25 PM Jelte Fennema-Nio wrote:
>
> On Fri, 8 Mar 2024 at 15:51, Peter Eisentraut wrote:
> > What is the relationship of these changes with the recently added
> > backtrace_on_internal_error?
>
> I think that's a reasonable question. And the follow up ones too.
>
> I
On Fri, 8 Mar 2024 at 15:51, Peter Eisentraut wrote:
> What is the relationship of these changes with the recently added
> backtrace_on_internal_error?
I think that's a reasonable question. And the follow up ones too.
I think it all depends on how close we consider
backtrace_on_internal_error
ldcard-support-to-backtrace_functions-GUC.patch
Description: Binary data
On 08.03.24 12:25, Jelte Fennema-Nio wrote:
On Fri, 8 Mar 2024 at 10:59, Alvaro Herrera wrote:
On 2024-Mar-08, Bharath Rupireddy wrote:
This works only if '* 'is specified as the only one character in
backtrace_functions = '*', right? If yes, what if someone sets
backtrace_functions = 'foo,
> On 8 Mar 2024, at 15:01, Bharath Rupireddy
> wrote:
> So, to get backtraces of all functions at
> backtrace_functions_min_level level, one has to specify
> backtrace_functions = '*'; combining it with function names is not
> allowed. This looks cleaner.
>
> postgres=# ALTER SYSTEM SET
On Fri, Mar 8, 2024 at 7:12 PM Daniel Gustafsson wrote:
>
> > On 8 Mar 2024, at 12:25, Jelte Fennema-Nio wrote:
> >
> > On Fri, 8 Mar 2024 at 10:59, Alvaro Herrera wrote:
> >>
> >> On 2024-Mar-08, Bharath Rupireddy wrote:
> >>
> >>> This works only if '* 'is specified as the only one character
> On 8 Mar 2024, at 12:25, Jelte Fennema-Nio wrote:
>
> On Fri, 8 Mar 2024 at 10:59, Alvaro Herrera wrote:
>>
>> On 2024-Mar-08, Bharath Rupireddy wrote:
>>
>>> This works only if '* 'is specified as the only one character in
>>> backtrace_functions = '*', right? If yes, what if someone sets
to be checked.
Makes sense. Attached is a new patchset that implements it that way.
I've not included Bharath his 0003 patch, since it's a much bigger
change than the others, and thus might need some more discussion.
v5-0002-Add-wildcard-support-to-backtrace_functions-GUC.patch
Description: Binary data
v5-0001-Add-backtrace_functions_min_level.patch
Description: Binary data
On 2024-Mar-08, Bharath Rupireddy wrote:
> This works only if '* 'is specified as the only one character in
> backtrace_functions = '*', right? If yes, what if someone sets
> backtrace_functions = 'foo, bar, *, baz'?
It throws an error, as expected. This is a useless waste of resources:
ne sets
backtrace_functions = 'foo, bar, *, baz'?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
v4-0001-Add-backtrace_functions_min_level.patch
Description: Binary data
v4-0002-Add-wildcard-support-to-backtrace_functions-GUC.patch
Description: Binary data
v4-0003-Simplify-backtrace_functions-GUC-code.patch
Description: Binary data
On 2024-Mar-06, Bharath Rupireddy wrote:
> +1 for disallowing *foo or foo* or foo*bar etc. combinations.
Cool.
> I think we need to go a bit further and convert backtrace_functions of
> type GUC_LIST_INPUT so that check_backtrace_functions can just use
> SplitIdentifierString to parse the list
On Thu, Feb 29, 2024 at 4:05 PM Alvaro Herrera wrote:
>
> Hmm, so if I write "foo,*" this will work but check all function names
> first and on the second entry. But if I write "foo*" the GUC value will
> be accepted but match nothing (as will "*foo" or "foo*bar"). I don't
> like either of
On 2024-Feb-28, Jelte Fennema-Nio wrote:
> diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
> index 699d9d0a241..553e4785520 100644
> --- a/src/backend/utils/error/elog.c
> +++ b/src/backend/utils/error/elog.c
> @@ -843,6 +843,8 @@ matches_backtrace_functions(const
> On 28 Feb 2024, at 19:50, Jelte Fennema-Nio wrote:
>
> On Wed, 28 Feb 2024 at 19:04, Daniel Gustafsson wrote:
>> This should be "equal to or higher" right?
>
> Correct, nicely spotted. Fixed that. I also updated the docs for the
> new backtrace_functions_min_level GUC itself too, as well as
Because when updating its docs
I realized that none of the existing level arrays matched what we
wanted.
From 5ab507155a1cb60e490209d28a092fc2d7c0f6be Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio
Date: Wed, 20 Dec 2023 11:41:18 +0100
Subject: [PATCH v4 2/2] Add wildcard support to backtrace_
> On 27 Feb 2024, at 18:03, Jelte Fennema-Nio wrote:
>
> On Mon, 12 Feb 2024 at 14:27, Jelte Fennema-Nio wrote:
>> Would a backtrace_functions_level GUC that would default to ERROR be
>> acceptable in this case?
>
> I implemented it this way in the attached patchset.
I'm not excited about
ennema-Nio
Date: Wed, 20 Dec 2023 11:41:18 +0100
Subject: [PATCH v3 2/2] Add wildcard support to backtrace_functions GUC
With this change setting backtrace_functions to '*' will start logging
backtraces for all errors (or more precisely all logs).
---
doc/src/sgml/config.sgml | 5
On 12.02.24 14:27, Jelte Fennema-Nio wrote:
And honestly wanting
to get backtraces for non-ERROR log entries seems quite niche to me,
which to me makes it a weird default.
I think one reason for this is that non-ERRORs are fairly unique in
their wording, so you don't have to isolate them by
On Mon, 12 Feb 2024 at 14:14, Daniel Gustafsson wrote:
> > The main problem it currently has is that it adds backtraces to all
> > LOG level logs too. So probably we want to make backtrace_functions
> > only log backtraces for ERROR and up (or maybe WARNING/NOTICE and up),
> > or add a
s, and restricting it now without a clear
usecase for doing so seems invasive (and someone might very well be using
this). 0001 in the attached adjusts this.
--
Daniel Gustafsson
v2-0002-Support-wildcard-in-backtrace_functions-to-handle.patch
Description: Binary data
v2-0001-doc-Clarify-when-bac
:18 +0100
Subject: [PATCH v1] Add wildcard support to backtrace_functions GUC
With this change setting backtrace_functions to '*' will start logging
backtraces for all errors (or more precisely all logs).
---
doc/src/sgml/config.sgml | 5 +
src/backend/utils/error/elog.c | 8 +---
2
59 matches
Mail list logo
