Re: straightening out backend process startup

At Mon, 13 Sep 2021 20:11:29 -0700, Andres Freund <andres@anarazel.de> wrote in 
> Hi,
> 
> On 2021-08-05 12:50:15 -0700, Andres Freund wrote:
> > On 2021-08-03 16:50:24 +0900, Kyotaro Horiguchi wrote:
> > > > - My first attempt at PostgresMainSingle() separated the single/multi user
> > > >   cases a bit more than the code right now, by having a PostgresMainCommon()
> > > >   which was called by PostgresMainSingle(), PostgresMain(). *Common only
> > > >   started with the MessageContext allocation, which did have the advantage of
> > > >   splitting out a few of the remaining conditional actions in PostgresMain()
> > > >   (PostmasterContext, welcome banner, Log_disconnections). But lead to a bit
> > > >   more duplication. I don't really have an opinion on what's better.
> > >
> > > I'm not sure how it looked like, but isn't it reasonable that quickdie
> > > and log_disconnections(). handle IsUnderPostmaster instead?  Or for
> > > log_disconnections, Log_disconnections should be turned off at
> > > standalone-initialization?
> > 
> > I was wondering about log_disconnections too. The conditional addition of the
> > callback is all that forces log_disconnections to be PGC_SU_BACKEND rather
> > than PGC_SUSET too. So I agree that moving a check for Log_disconnections and
> > IsUnderPostmaster into log_disconnections is a good idea.
> 
> I did that, and it didn't seem a clear improvement..

Sorry for bothering you about that..

> > > > - I had to move the PgStartTime computation to a bit earlier for single user
> > > >   mode. That seems to make sense to me anyway, given that postmaster does so
> > > >   fairly early too.
> > > >
> > > >   Any reason that'd be a bad idea?
> > > >
> > > >   Arguably it should even be a tad earlier to be symmetric.
> > >
> > > Why don't you move the code for multiuser as earlier as standalone does?
> > 
> > I think it's the other way round - right now the standalone case is much later
> > than the multiuser case. Postmaster determines PgStartTime after creating
> > shared memory, just before starting checkpointer / startup process - whereas
> > single user mode in HEAD does it just before accepting input for the first
> > time.
> 
> Did that in the attached.
> 
> 
> I've attached the three remaining patches, after some more polish. Unless
> somebody argues against I plan to commit these soon-ish.

0001 looks fine.

0002: Looks fine in conjunction with 0003.

0003:

PostgresSingleUserMain doesn't set processing mode. It is fine as it
is initialied to InitProcessing at process start.  On the other hand,
PostgresMain sets it to InitProcessing but it seems to me it is always
InitProcessing when entering the function. In the first place isn't
InitProcessing the initial state and won't be transitioned from other
states?  However, it would be another issue even if it is right.

So, everything looks fine to me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center