Re: PANIC serves too many masters

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-18 17:12:57
Message-ID: CAJTYsWVwhoY-6S8ExQ4QfBtKNiiQd2+skYt_ogibbH_06-gTHg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Sat, 12 Sept 2026 at 21:50, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
wrote:
>
> 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?

Attached is v2, which sets io_method=worker for the TAP test.

The repeated crash/recovery cycles hit the unrelated io_uring resource
leak discussed at [1] in CFBot. Using worker avoids that failure; the
rest of the code is unchanged from v1.

[1]
https://www.postgresql.org/message-id/flat/zHN8Rb7syWYDBPUhCICxKLr_tQmFLg0T024irxVVb6RSSicZcqcA0eT73z2PQy7JmzzqnUzX0Hi3P2dPKRvmNx_jbAqSCLyqJvWjXEypyAI%3D%40draescher.fr

Regards,
Ayush

Attachment Content-Type Size
v2-0002-Avoid-core-dumps-for-WAL-disk-full-failures.patch application/octet-stream 8.8 KB
v2-0001-Allow-selected-PANIC-errors-to-skip-core-dumps.patch application/octet-stream 22.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2026-09-18 17:15:59 Re: Regression tests failures due to concurrent grants
Previous Message Andres Freund 2026-09-18 17:11:46 Re: Regression tests failures due to concurrent grants