MIME-Version: 1.0
References: 
 <CAJTYsWVoD3V9yhhqSae1_wqcnTdpFY-hDT7dPm5005ZFsL_bpA@mail.gmail.com>
 <0f462532-9790-4334-b503-4ee522225820@iki.fi>
 <CAJTYsWVw3h1hTDyUdaDb8MmmXF0qQKzV3YkQUwC1DhvcApFvTw@mail.gmail.com>
 <CAJTYsWX=EkwTvA15=T8dE1c4iVoapTT5=nkrByd2h-18NU8hZA@mail.gmail.com>
 <aeK6Fj9TiQWCcFgN@paquier.xyz>
In-Reply-To: <aeK6Fj9TiQWCcFgN@paquier.xyz>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Mon, 20 Apr 2026 12:11:45 +0530
Message-ID: 
 <CAJTYsWUEm1h_k+4Wwnri+n+V8MGPSAeyU27ehH+YwiFjJwYRvA@mail.gmail.com>
Subject: Re: [PATCH] postmaster: fix stale PM_STARTUP comment
To: Michael Paquier <michael@paquier.xyz>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@postgresql.org,
	"noah@leadboat.com" <noah@leadboat.com>
Content-Type: multipart/alternative; boundary="00000000000041973f064fde9845"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWUEm1h_k%2B4Wwnri%2Bn%2BV8MGPSAeyU27ehH%2BYwiFjJwYRvA%40mail.gmail.com>
Precedence: bulk

--00000000000041973f064fde9845
Content-Type: text/plain; charset="UTF-8"

Hi,

Thanks a lot for the review!.

On Sat, 18 Apr 2026 at 04:24, Michael Paquier <michael@paquier.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

--00000000000041973f064fde9845
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,=C2=A0<br><br>Thanks a lot for the rev=
iew!.<br><div><br></div></div><div class=3D"gmail_quote gmail_quote_contain=
er"><div dir=3D"ltr" class=3D"gmail_attr">On Sat, 18 Apr 2026 at 04:24, Mic=
hael Paquier &lt;<a href=3D"mailto:michael@paquier.xyz">michael@paquier.xyz=
</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:=
0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">=
On Fri, Apr 17, 2026 at 07:17:18PM +0530, Ayush Tiwari wrote:<br>
&gt; I&#39;ve attached a patch, please review and let me know your thoughts=
.<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/*<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* Unexpected exit of start=
up process (including FATAL exit)<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* during PM_STARTUP is tre=
ated as catastrophic. There are no<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* other processes running =
yet, so we can just exit.<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (pmState =3D=3D PM_STARTUP &a=
mp;&amp;<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 StartupStatus !=3D=
 STARTUP_SIGNALED &amp;&amp;<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !EXIT_STATUS_0(exi=
tstatus))<br>
<br>
The assumption that only the startup process is running while we are<br>
in a PM_STARTUP state is wrong since 7ff23c6d277d, and this exit code<br>
path has been missed the fact that now the checkpointer and the<br>
bgwriter are started during recovery.=C2=A0 So this means a backpatch.<br><=
/blockquote><div><br></div><div>Agreed, this is latent since 7ff23c6d277d (=
v15). I can prepare a back-branch=C2=A0</div><div>versions patch for v15..m=
aster once the master patch shape is settled<br>=C2=A0</div><blockquote cla=
ss=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid =
rgb(204,204,204);padding-left:1ex">
<br>
Removing the assertion is the right move, yes, there are other<br>
children, so again that&#39;s an issue with 7ff23c6d277d.=C2=A0 I am planni=
ng<br>
to double-check the shutdown sequence while switching to<br>
PM_WAIT_BACKENDS under a PM_STARTUP, just note that bgworkers that use<br>
BgWorkerStart_PostmasterStart cannot request a database connection,<br>
meaning that PM_WAIT_BACKENDS should be pointeless, but perhaps it<br>
makes the whole shutdown flow easier to reason about, as you are<br>
suggesting, as making this path more complicated would lead to the<br>
addition of more postmaster states.=C2=A0 Making this code simpler, not<br>
more complicated, is always useful.<br>
--<br>
Michael<br></blockquote><div><br></div>Right, I believe at PM_STARTUP the o=
nly children we can possibly have are</div><div class=3D"gmail_quote gmail_=
quote_container">the auxiliary processes (checkpointer, bgwriter, walwriter=
=C2=A0(if applicable),</div><div class=3D"gmail_quote gmail_quote_container=
">=C2=A0IO workers) and BgWorkerStart_PostmasterStart bgworkers,=C2=A0</div=
><div class=3D"gmail_quote gmail_quote_container">none of which should hold=
 backend slots. So, PM_WAIT_BACKENDS is functionally=C2=A0</div><div class=
=3D"gmail_quote gmail_quote_container">a no-op in this case and we transiti=
on straight through to=C2=A0</div><div class=3D"gmail_quote gmail_quote_con=
tainer">PM_WAIT_DEAD_END.<br><br><div>I considered short-circuiting directl=
y to=C2=A0</div><div>PM_WAIT_DEAD_END from PM_STARTUP, but routing through =
the same=C2=A0</div><div>state path the rest of the crash-handling code use=
s keeps the state machine=C2=A0</div><div>uniform and avoids a special case=
 in HandleFatalError() / PostmasterStateMachine().=C2=A0</div><div>Making t=
he code simpler, as you mentioned.<br><br>Regards,<br>Ayush<br></div></div>=
</div>

--00000000000041973f064fde9845--