Re: [PATCH] Clear FatalError earlier during crash restart - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Clear FatalError earlier during crash restart
Date
Msg-id arxfsNVW0rNWYui8@paquier.xyz
Whole thread
In response to Re: [PATCH] Clear FatalError earlier during crash restart  (Ayush Tiwari <ayushtiwari.slg01@gmail.com>)
Responses Re: [PATCH] Clear FatalError earlier during crash restart
List pgsql-hackers
On Wed, Sep 30, 2026 at 05:44:16AM +0530, Ayush Tiwari wrote:
> I think that argues for the smaller fix, at least on the back branches.
> I'd tried the diff below[1] before clearing FatalError: it SIGQUITs the new
> children on smart/fast shutdown, without changing the crash-restart
> behavior. It fixed my repro on 46024c573bc, though it logs an abnormal
> shutdown.

Yeah, I can see that, due to this part I think because the FatalError
flag would be set?  Quoting the relevant part from postmaster.c:
    if (Shutdown > NoShutdown && pmState == PM_NO_CHILDREN)
    {
        if (FatalError)
        {
            ereport(LOG, (errmsg("abnormal database system shutdown")));
            ExitPostmaster(1);
        }
        else
        {
            /*
             * Normal exit from the postmaster is here.  We don't need to log
             * anything here, since the UnlinkLockFiles proc_exit callback
             * will do so, and that should be the last user-visible action.
             */
            ExitPostmaster(0);
        }
    }

So that just feels, err, okay-ish, because the logs reflect what we
are actually doing?

So plugging in an extra HandleFatalError() while the pmState is
PM_STOP_BACKENDS feels like a good compromise, on top of my mind.  It
would mean that your too-early-shutdown request on crash recovery
would still persist on v17 and older branches, but at least the
checkpointer and the I/O workers would be able to understand that they
need to stop, and the SIGQUIT sent would be promoted to a SIGKILL
after the AbortStartTime timeout is armed.

I was slightly on the edge about your TAP test and its fancy restore
command, but I don't want to leave that untested, either, as that
seems like the same thing as Justin's case.  Perhaps only do that on
HEAD first, let it brew for a bit, and consider it down later on if
really necessary?  These shutdown changes stress me quite a bit when
it comes to a backpatch, because they can be nasty very easily with
one mistake, and I like a peaceful sleep.  On top of that v19 is close
by.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Temporary slot leak when creation fails in a subtransaction
Next
From: Michael Paquier
Date:
Subject: Re: ReplicationSlotRelease() clobbers another backend's statusFlags entry