Re: ReadRecentBuffer() doesn't scale well

Hi,

On 2025-10-08 13:39:14 +1300, Thomas Munro wrote:
> On Fri, Sep 19, 2025 at 11:44 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > On Fri, Sep 19, 2025 at 12:36 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > I'm planning to commit 0001 soon, unless you'd like to do the honors - I would
> > > break it with some upcoming patches, and it's a good improvement. Those
> > > patches also will PinBuffer_Locked() a bit slower, i.e. it'd be good to avoid
> > > using it in ReadRecentBuffer() for that reason alone.
> >
> > Oh, thanks for thinking about that interaction. I'll go ahead and
> > push it later today after I re-convince myself that it's correct.
>
> Sorry I haven't got to this yet. Please feel free to go ahead and
> push it if it's blocking you...

Done after two small changes:

- NewPrivateRefCountEntry() has to be deferred until after the added return
false in PinBuffer(), otherwise a failed ReadRecentBuffer() would leave a
refcount entry arround, triggering a CheckBufferLeaks() failure

Found via test_aio. Interestingly I had to do the same change for unrelated
reasons in one of the other patches that I am working on, which hid this
issue for a while...

- Moved the return false in PinBuffer() to before the WaitBufHdrUnlocked(), it
seems a bit silly to wait for the lock and then return.

We now should really again look at your patch to make btree searches cache the
root page buffer, the wins for that were rather large...

Greetings,

Andres Freund