Hi,
On Fri, Jan 24, 2025 at 03:06:17PM -0500, Andres Freund wrote:
> On 2025-01-15 08:38:33 +0000, Bertrand Drouvot wrote:
> > Fair enough. I'll look at the remaining pieces once this stuff goes in.
>
> Cool. I did try writing the change, but it does indeed seem better as a
> separate patch. Besides the error message, it also seems we ought to invent a
> different ERRCODE, as neither ERRCODE_ADMIN_SHUTDOWN nor
> ERRCODE_CRASH_SHUTDOWN seem appropriate.
Thanks for the feedback, will look at it.
> Vaguely related and not at all crucial:
>
> quickdie(), in the PMQUIT_FOR_CRASH, does
> errhint("In a moment you should be
> able to reconnect to the"
> " database and repeat
> your command.")));
>
> but in case of an immediate *restart* (via pg_ctl restart -m immediate) we'll
> not have such hint. postmaster.c doesn't know it's an immediate restart, so
> that's not surprising, but it still seems a bit weird. One would hope that
> immediate restarts are more common than crashes...
Yeah, it does not make the distinction between an "immediate restart" and an
"immediate stop". I agree that such hint "should" be added in case of "immediate
restart" (and keep "immediate stop" as it is): will give it a look.
> >
> > > > But that would also need to call TerminateChildren() (a second time)
> > > > again after
> > > > ConfigurePostmasterWaitSet() which is not ideal though (unless we move
> > > > the
> > > > TerminateChildren() calls in the switch or add a check on
> > > > PM_WAIT_DEAD_END in
> > > > the first call).
> > >
> > > Why would it need to?
> >
> > Because I thought that TerminateChildren() needs to be called after
> > ConfigurePostmasterWaitSet(false). If not, could new connections be
> > accepted in
> > the window between TerminateChildren() and
> > ConfigurePostmasterWaitSet(false)?
>
> I don't think that can happen with the attached patch - the
> ConfigurePostmasterWaitSet() is called before HandleFatalError() returns, so
> no new connections can be accepted, as that happens only from
> ServerLoop()->AcceptConnection().
Thanks for the explanation, that was the missing part in my reasoning. So yeah,
all good as no new connections are accepted.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
