Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, tender wang <tndrwang(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock
Date: 2024-02-06 11:35:16
Message-ID: CAFiTN-v5qnikURw2Trsds11iXiwurFiDc87-k2t44qD3rQH+FQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 6, 2024 at 4:23 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> > > (We also have SimpleLruTruncate, but I think it's not as critical to
> > > have a barrier there anyhow: accessing a slightly outdated page number
> > > could only be a problem if a bug elsewhere causes us to try to truncate
> > > in the current page. I think we only have this code there because we
> > > did have such bugs in the past, but IIUC this shouldn't happen anymore.)
> >
> > +1, I agree with this theory in general. But the below comment in
> > SimpleLruTrucate in your v3 patch doesn't seem correct, because here
> > we are checking if the latest_page_number is smaller than the cutoff
> > if so we log it as wraparound and skip the whole thing and that is
> > fine even if we are reading with atomic variable and slightly outdated
> > value should not be a problem but the comment claim that this safe
> > because we have the same bank lock as SimpleLruZeroPage(), but that's
> > not true here we will be acquiring different bank locks one by one
> > based on which slotno we are checking. Am I missing something?
>
> I think you're correct. I reworded this comment, so now it says this:
>
> /*
> * An important safety check: the current endpoint page must not be
> * eligible for removal. This check is just a backstop against wraparound
> * bugs elsewhere in SLRU handling, so we don't care if we read a slightly
> * outdated value; therefore we don't add a memory barrier.
> */
>
> Pushed with those changes. Thank you!

Yeah, this looks perfect, thanks.

> Now I'll go rebase the rest of the patch on top.

Okay, I will review and test after that.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2024-02-06 11:37:44 Re: Synchronizing slots from primary to standby
Previous Message Amit Kapila 2024-02-06 11:21:02 Re: Synchronizing slots from primary to standby