Re: straightening out backend process startup

Hi,

On 2021-08-03 16:50:24 +0900, Kyotaro Horiguchi wrote:
> At Mon, 2 Aug 2021 09:41:24 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> > - PostgresMainSingle() should probably not be in postgres.c. We could put it
> > into postinit.c or ..?
>
> PostgresMainSingle() looks like the single-process version of
> PostgresMain so it is natural that they are placed together in
> postgres.c. If PostgresMainSingle is constructed as initializing
> standalone first then calling PostgresMain, it might be right that
> PostgresMain calls the initialization function resides in postinit.c
> if !IsUnderPostmaster.
>
> PostgresMain()
> {
> if (!IsUnderPostmaster)
> InitSinglePostgres(argv[0]);
> ...

I find passing argc/argv to functions that don't actually need them, like
PostgresMain during normal operation, confusing. Especially when the argc/argv
values are just manufactured stuff like in the case of PostgresMain(). So I
think it's better to have have the order work the other way round.

> > - 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 don't understand your reference to quickdie() though?

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

Greetings,

Andres Freund