Re: PANIC serves too many masters

Hi,

On 2023-11-20 17:55:32 -0500, Tom Lane wrote:
> Jeff Davis <pgsql(at)j-davis(dot)com> writes:
> > Is the error level the right way to express what we want to happen? It
> > seems like what we really want is to decide on the behavior, i.e.
> > restart or not, and generate core or not. That could be done a
> > different way, like:
>
> > ereport(PANIC,
> > (errmsg("could not locate a valid checkpoint record"),
> > errabort(false),errrestart(false)));
>
> Yeah, I was wondering about that too. It feels to me that
> PANIC_EXIT is an error level (even more severe than PANIC).
> But maybe "no core dump please" should be conveyed separately,
> since it's just a minor adjustment that doesn't fundamentally
> change what happens.

I guess I was thinking of an error level because that'd be easier to search
for in logs. It seems reasonable to want to specificially search for errors
that cause core dumps, since IMO they should all be "should never happen" kind
of paths.

> It's plausible that you'd want a core,
> or not want one, for different cases that all seem to require
> PANIC_EXIT.

I can't immediately think of a case where you'd want PANIC_EXIT but also want
a core dump? In my mental model to use PANIC_EXIT we'd need to have a decent
understanding that the situation isn't going to change after crash-restart -
in which case a core dump presumably isn't interesting?

> (Need a better name than PANIC_EXIT. OMIGOD?)

CRITICAL?

I agree with the point made upthread that we'd want leave PANIC around, it's
not realistic to annotate everything, and then there's obviously also
extensions (although I hope there aren't many PANICs in extensions).

If that weren't the case, something like this could make sense:

PANIC: crash-restart
CRITICAL: crash-shutdown
BUG: crash-restart, abort()

Greetings,

Andres Freund