Hi, On September 13, 2021 9:56:52 PM PDT, Kyotaro Horiguchi <horikyota....@gmail.com> wrote: >At Mon, 13 Sep 2021 20:11:29 -0700, Andres Freund <and...@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.
