Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()

At Wed, 13 Oct 2021 13:39:24 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
> On Wed, Oct 13, 2021 at 4:25 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > Hi,
> >
> > On 2021-10-11 15:24:46 +0900, Fujii Masao wrote:
> > > How about modifying SharedInvalBackendInit() so that it accepts
> > > BackendId as an argument and allocates the ProcState entry of
> > > the specified BackendId? That is, the startup process determines
> > > that its BackendId is "MaxBackends + MyAuxProcType (=StartupProcess) + 1"
> > > in AuxiliaryProcessMain(), and then it passes that BackendId to
> > > SharedInvalBackendInit() in InitRecoveryTransactionEnvironment().
> >
> > If I understand correctly what you're proposing, I think that's going in the
> > wrong direction. We should work towards getting rid of BackendIds
> > instead. This whole complication vanishes if we make sinvaladt use pgprocno.
> >
> > See https://postgr.es/m/20210802171255.k4yv5cfqaqbuuy6f%40alap3.anarazel.de
> > for some discussion of this.

I feel this is the right direction. I understand sinvaladt needs the
backend id but I think it's wrong that the backend id is used widely.
And, in the current direction, procState array gets very sparse and
performence of sinval gets degraded.

> Will any of the backends get pgprocno greater than MaxBackends? The
> pgprocno can range from 0 to ((MaxBackends + NUM_AUXILIARY_PROCS +
> max_prepared_xacts) - 1) and the ProcState array size is MaxBackends.
> How do we put a backend with pgprocno > MaxBackends, into the
> ProcState array? Is it that we also increase ProcState array size to
> (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts)? Probably
> this is the dumbest thing as some slots are unused
> (NUM_AUXILIARY_PROCS - 1 + max_prepared_xacts slots. -1 because the
> startup process needs a ProcState slot) and the shared memory is
> wasted.

The elements for max_prepared_xacts can be placed at the last part of
pgprocno and we can actually omit allocating memory for them. I don't
think NUM_AUXILIARY_PROC is that large. It's only 5 and won't increase
much in future. Since we know that startup is the only user of
procsig, we can save the space for four of them. In short, no extra
memory is required by using pgprocno for now.

The actual problem is if we simply use pgprocno in sinvaladt, we will
see preformance degradation caused by the super sparse procState
array. As Andres mentioned in the URL above, it can be avoided if we
can get rid of looping over the array.

Although needing a bit of care for the difference of invalid values
for both though, BackendId can be easily replaced with pgprocno almost
mechanically except sinvaladt. Therefore, we can confine the current
backend ID within sinvaladt isolating from other part. The ids
dedicated for sinvaladt can be packed to small range and perfomance
won't be damaged.

In the future, if we can get rid of looping over the procState array,
sinvaladt - the last user of the current backend ID - can move to
pgprocno and we will say good-bye to the current backend ID.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center