public inbox for [email protected]  

help / color / mirror / Atom feed

From: Ayush Tiwari <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: [email protected]
Cc: [email protected] <[email protected]>
Subject: Re: [PATCH] postmaster: fix stale PM_STARTUP comment
Date: Mon, 20 Apr 2026 12:11:45 +0530
Message-ID: <CAJTYsWUEm1h_k+4Wwnri+n+V8MGPSAeyU27ehH+YwiFjJwYRvA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAJTYsWVoD3V9yhhqSae1_wqcnTdpFY-hDT7dPm5005ZFsL_bpA@mail.gmail.com>
	<[email protected]>
	<CAJTYsWVw3h1hTDyUdaDb8MmmXF0qQKzV3YkQUwC1DhvcApFvTw@mail.gmail.com>
	<CAJTYsWX=EkwTvA15=T8dE1c4iVoapTT5=nkrByd2h-18NU8hZA@mail.gmail.com>
	<[email protected]>

Hi,

Thanks a lot for the review!.

On Sat, 18 Apr 2026 at 04:24, Michael Paquier <[email protected]> wrote:

> On Fri, Apr 17, 2026 at 07:17:18PM +0530, Ayush Tiwari wrote:
> > I've attached a patch, please review and let me know your thoughts.
>
>              /*
> -             * Unexpected exit of startup process (including FATAL exit)
> -             * during PM_STARTUP is treated as catastrophic. There are no
> -             * other processes running yet, so we can just exit.
> -             */
> -            if (pmState == PM_STARTUP &&
> -                StartupStatus != STARTUP_SIGNALED &&
> -                !EXIT_STATUS_0(exitstatus))
>
> The assumption that only the startup process is running while we are
> in a PM_STARTUP state is wrong since 7ff23c6d277d, and this exit code
> path has been missed the fact that now the checkpointer and the
> bgwriter are started during recovery.  So this means a backpatch.
>

Agreed, this is latent since 7ff23c6d277d (v15). I can prepare a
back-branch
versions patch for v15..master once the master patch shape is settled


>
> Removing the assertion is the right move, yes, there are other
> children, so again that's an issue with 7ff23c6d277d.  I am planning
> to double-check the shutdown sequence while switching to
> PM_WAIT_BACKENDS under a PM_STARTUP, just note that bgworkers that use
> BgWorkerStart_PostmasterStart cannot request a database connection,
> meaning that PM_WAIT_BACKENDS should be pointeless, but perhaps it
> makes the whole shutdown flow easier to reason about, as you are
> suggesting, as making this path more complicated would lead to the
> addition of more postmaster states.  Making this code simpler, not
> more complicated, is always useful.
> --
> Michael
>

Right, I believe at PM_STARTUP the only children we can possibly have are
the auxiliary processes (checkpointer, bgwriter, walwriter (if applicable),
 IO workers) and BgWorkerStart_PostmasterStart bgworkers,
none of which should hold backend slots. So, PM_WAIT_BACKENDS is
functionally
a no-op in this case and we transition straight through to
PM_WAIT_DEAD_END.

I considered short-circuiting directly to
PM_WAIT_DEAD_END from PM_STARTUP, but routing through the same
state path the rest of the crash-handling code uses keeps the state machine
uniform and avoids a special case in HandleFatalError() /
PostmasterStateMachine().
Making the code simpler, as you mentioned.

Regards,
Ayush

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] postmaster: fix stale PM_STARTUP comment
  In-Reply-To: <CAJTYsWUEm1h_k+4Wwnri+n+V8MGPSAeyU27ehH+YwiFjJwYRvA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox