| 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
| 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 |