Re: Support a wildcard in backtrace_functions - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: Support a wildcard in backtrace_functions |
Date | |
Msg-id | CA+Tgmoa_+t8mkqkYAFW9fqATAu3pLfV_Ez+EQNUQbOQjAX0tGA@mail.gmail.com Whole thread Raw |
In response to | Re: Support a wildcard in backtrace_functions (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Support a wildcard in backtrace_functions
|
List | pgsql-hackers |
On Fri, Apr 19, 2024 at 2:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> 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 certain that nobody will put SETs of this GUC > into their applications, or even just activate it via ALTER DATABASE > SET? If they've done that, then a GUC change means dump/reload/upgrade > failure. That's fair. That feature isn't super-widely used, but it wouldn't be crazy for someone to use it with this feature, either. > 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 altogether, and try again in v18. When we're still > designing something after feature freeze, that is a good indication > that we are trying to ship something that's not ready for prime time. There are two features at issue here. One is backtrace_on_internal_error, committed as a740b213d4b4d3360ad0cac696e47e5ec0eb8864. The other is a feature to produce backtraces for all errors, which was originally proposed as backtrace_functions='*', backtrace_functions_level=ERROR but which has subsequently been proposed with a few other spellings that involve merging that functionality into backtrace_on_internal_error. To the extent that there's an open question here for PG17, it's not about reverting this patch (which AIUI has never been committed) but about reverting the earlier patch for backtrace_on_internal_error. So is that the right thing to do? Well, on the one hand, I confess to having had a passing thought that backtrace_on_internal_error is awfully specific. Surely, user A's criterion for which messages should have backtraces might be anything, and we cannot reasonably add backtrace_on_$WHATEVER for all $WHATEVER in some large set. And the debate here suggests that I wasn't the only one to have that concern. So that argues for a revert. 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. I was disappointed to see it added as a developer option with GUC_NOT_IN_SAMPLE. My vote would have been to put it in postgresql.conf and enable it by default. We have a somewhat debatable habit of using the exact same message in many places with similar kinds of problems, and when a production system manages to hit one of those errors, figuring out what actually went wrong is hard. In fact, it's often hard even when the error text only occurs in one or two places, because it's often some very low-level part of the code where you can't get enough context to understand the problem without knowing where that code got called from. So I sort of hate to see one of the most useful extensions of backtrace functionality that I can personally imagine get pulled back out of the tree because it turns out that someone else has something else that they want. I wonder whether a practical solution here might be to replace backtrace_on_internal_error=true|false with backtrace_on_error=true|internal|false. (Yes, I know that more proposed resolutions is not necessarily what we need right now, but I can't resist floating the idea.) -- Robert Haas EDB: http://www.enterprisedb.com
pgsql-hackers by date: