Re: [Patch] Optimize dropping of relation buffers using dlist

At Tue, 22 Dec 2020 08:08:10 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote in
> On Tue, Dec 22, 2020 at 7:13 AM tsunakawa(dot)takay(at)fujitsu(dot)com
> <tsunakawa(dot)takay(at)fujitsu(dot)com> wrote:
> >
> > From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> > > This answers the second part of the question but what about the first
> > > part (We hold a buffer partition lock, and have done a lookup in th
> > > mapping table. Why are we then rechecking the
> > > relfilenode/fork/blocknum?)
> > >
> > > I think we don't need such a check, rather we can have an Assert
> > > corresponding to that if-condition in the patch. I understand it is
> > > safe to compare relfilenode/fork/blocknum but it might confuse readers
> > > of the code.
> >
> > Hmm, you're right. I thought someone else could steal the found buffer and use it for another block because the buffer mapping lwlock is released without pinning the buffer or acquiring the buffer header spinlock.
> >
>
> Okay, I see your point.
>
> > However, in this case (replay of TRUNCATE during recovery), nobody steals the buffer: bgwriter or checkpointer doesn't use a buffer for a new block, and the client backend waits for AccessExclusive lock.
> >
> >

I understood that you are thinking that the rechecking is useless.

> Why would all client backends wait for AccessExclusive lock on this
> relation? Say, a client needs a buffer for some other relation and
> that might evict this buffer after we release the lock on the
> partition. In StrategyGetBuffer, it is important to either have a pin
> on the buffer or the buffer header itself must be locked to avoid
> getting picked as victim buffer. Am I missing something?

I think exactly like that. If we acquire the bufHdr lock before
releasing the partition lock, that steal doesn't happen but it doesn't
seem good as a locking protocol.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center