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

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: andres(at)anarazel(dot)de
Cc: bharath(dot)rupireddyforpostgres(at)gmail(dot)com, masao(dot)fujii(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Date: 2021-10-15 06:00:57
Message-ID: 20211015.150057.1538271214250441184.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message torikoshia 2021-10-15 06:17:22 Re: RFC: Logging plan of the running query
Previous Message Greg Nancarrow 2021-10-15 04:30:37 Re: row filtering for logical replication