Re: straightening out backend process startup

Hi, 

On September 13, 2021 9:56:52 PM PDT, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>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 thought it might look better too. Might still be worth later, just to make the guc SUSET.


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

I don't think it should be accessed during the stuff that PostgresSingleUserMain() does. The catalog access / cache initialization continues to happen below PostgresMain(). I guess a comment wouldn't me amiss.

I think we have way too many different process type & state variables :(

Andres



-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.