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
>
On Fri, 22 Mar 2024 at 11:14, Bharath Rupireddy
wrote:
> 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
On Thu, Mar 21, 2024 at 8:11 PM Jelte Fennema-Nio 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
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
On Fri, 8 Mar 2024 at 17:17, Bharath Rupireddy
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
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
On Fri, 8 Mar 2024 at 14:42, Daniel Gustafsson wrote:
> My documentation comments from upthread still
> stands, but other than those this version LGTM.
Ah yeah, I forgot about those. Fixed now.
v6-0001-Add-backtrace_functions_min_level.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
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, bar, *, baz'?
>
> It throws an
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:
On Wed, Mar 6, 2024 at 12:41 AM Alvaro Herrera wrote:
>
> > 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 of identifiers. Then, the
> > strspn can just be
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
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 creating
a dedicated options array for the GUC. Because when
> 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
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.
From 71e2c1f1fa903ecfce4a79ff5981d0d754d134a2 Mon Sep 17 00:00:00 2001
From: Jelte
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
> On 20 Dec 2023, at 12:23, Jelte Fennema-Nio wrote:
> Attached is a trivial patch that starts supporting
> backtrace_functions='*'. By setting that in postgresql.conf for my dev
> environment it starts logging backtraces always.
I happened to implement pretty much the same diff today during a
58 matches
Mail list logo
