| From: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
|---|---|
| To: | Jeff Davis <pgsql(at)j-davis(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: PANIC serves too many masters |
| Date: | 2026-09-12 16:20:31 |
| Message-ID: | CAJTYsWV601xS0D_SrZ111VX0N+tRe2fjK-VjY75v1gNJ93+jkQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
(Coming back to this after looking at a bunch of core dumps from
disk-full PANICs.)
On Sat, 12 Sept 2026 at 03:10, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> On Mon, 2023-11-20 at 17:12 -0500, Tom Lane wrote:
> > I'd be inclined to keep PANIC with its current meaning, and
> > incrementally change call sites where we decide that's not the
> > best behavior. I think those will be a minority, maybe a small
> > minority. (PANIC_EXIT had darn well better be a small minority.)
>
> 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)));
I gave the core-dump part of this a try. Two WIP patches attached.
I first thought of adding PANIC_NO_CORE, but wasn't sure where to put
it. Below PANIC, we'd have to adjust checks like elevel >= PANIC.
Above PANIC, it could take precedence over an ordinary PANIC during
nested error reporting. Neither seemed quite right when all I wanted
was to avoid the core dump.
Following your suggestion, I kept PANIC and added a flag to ErrorData.
The call sites use errnocoredump_on_errno(ENOSPC), which checks the
saved errno. For a marked PANIC, errfinish() takes the _exit(2) path
instead of calling abort(). There's also a developer GUC to get the
abort() behavior back when needed. (may not be needed?)
The second patch adds this to selected WAL and pg_control error paths.
I haven't changed the restart policy in any of them, patch is just for the
core dump behaviour.
Would something along these lines make sense?
Regards,
Ayush
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Allow-selected-PANIC-errors-to-skip-core-dumps.patch | application/octet-stream | 22.7 KB |
| v1-0002-Avoid-core-dumps-for-WAL-disk-full-failures.patch | application/octet-stream | 8.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Ayush Tiwari | 2026-09-12 15:50:02 | Re: BUG #19631: currtid2() on a view with GROUP BY ctid crashes with XX000 |