Re: [PATCH] postmaster: fix stale PM_STARTUP comment

From: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers(at)postgresql(dot)org, "noah(at)leadboat(dot)com" <noah(at)leadboat(dot)com>
Subject: Re: [PATCH] postmaster: fix stale PM_STARTUP comment
Date: 2026-04-20 06:41:45
Message-ID: CAJTYsWUEm1h_k+4Wwnri+n+V8MGPSAeyU27ehH+YwiFjJwYRvA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks a lot for the review!.

On Sat, 18 Apr 2026 at 04:24, Michael Paquier <michael(at)paquier(dot)xyz> 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-04-20 06:46:09 Re: EXCEPT TABLE - Case inconsistency for describe \d and \dRp+
Previous Message Chengpeng Yan 2026-04-20 06:17:47 Re: [PATCH] Fix hashed ScalarArrayOp semantics for NULL LHS with non-strict comparators