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

On Fri, Oct 15, 2021 at 11:31 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Thu, 14 Oct 2021 10:53:06 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> > Hi,
> >
> > On 2021-10-14 17:28:34 +0900, Kyotaro Horiguchi wrote:
> > > At Wed, 13 Oct 2021 19:52:52 +0900 (JST), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> > > > 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.
> >
> > FWIW, I don't actually think there's necessarily that strong a need for
> > density in sinvaladt. With a few relatively changes we can get rid of the O(n)
> > work in the most crucial paths.
>
> Right. So I left it for the "future:p
>
> > In https://www.postgresql.org/message-id/20210802171255.k4yv5cfqaqbuuy6f%40alap3.anarazel.de
> > I wrote:
> > > Another approach to deal with this could be to simply not do the O(n) work in
> > > SIInsertDataEntries(). It's not obvious that ->hasMessages is actually
> > > necessary - we could atomically read maxMsgNum without acquiring a lock
> > > instead of needing the per-backend ->hasMessages. I don't the density would
> > > be a relevant factor in SICleanupQueue().
> >
> > This'd get rid of the need of density *and* make SIInsertDataEntries()
> > cheaper.
>
> Yes. So.. I tried that. The only part where memory-flush timing is
> crucial seems to be between writing messages and setting maxMsgNum.
> By placing memory barrier between them it seems *to me* we can read
> maxMsgNum safely without locks.
>
> I reread that thread and found we can get rid of O(N) behavior from
> two places, SIgnalVirtualTransaction and GetVirtualXIDsDelayingChkpt.
>
> Finally, I got rid of the siindex (the old BackendId) from sinvaladt.c
> at all. Still CleanupInvalidationState and SICleanupQueue has O(N)
> behavior but they are executed rarely or ends in a short time in the
> most cases.
>
>
> 0001: Reverses the proc freelist so that the backendid is assigned in
> the sane order.
>
> 0002: Replaces the current BackendId - that is generated by
> sinvaladt.c intending to pack the ids to a narrow range - with
> pgprocno in most of the tree. The old BackendID is now used only in
> sinvaladt.c
>
> 0003: Removes O(N) behavior from SIInsertDataEntries. I'm not sure it
> is correctly revised, though..
>
> 0004: Gets rid of O(N), or reduce O(N^2) to O(N) of
> HaveVirtualXIDsDelayingChkpt and SignalVirtualTransaction.
>
> 0005: Gets rid of the old BackendID at all from sinvaladt.c.

Hi,

I'm not sure if the above approach and the patches are okay that I or
someone can start reviewing them. Does anyone have comments on this
please?

Regards,
Bharath Rupireddy.