Re: SLRUs in the main buffer pool - Page Header definitions

From: "Bagga, Rishu" <bagrishu(at)amazon(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, "Debnath, Shawn" <sdn(at)amazon(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>
Subject: Re: SLRUs in the main buffer pool - Page Header definitions
Date: 2023-02-08 20:04:52
Message-ID: BBA4E674-ABCC-4788-AE1C-8EB295B217FE@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Unfortunately a quite large patch, with this little explanation, is
> hard to review. I could read through the entire thread to try to
> figure out what this is doing, but I really shouldn't have to.

> You're changing quite fundamental APIs across the tree. Why is that
> required for the topic at hand? Why is it worth doing that, given
> it'll break loads of extensions?

Hi Andres,

Thanks for your response.

To summarize, our underlying effort is to move the SLRUs to the buffer
cache. We were working with Thomas Munro off a patch he introduced here
[1]. Munro’s patch moves SLRUs to the buffer cache by introducing the
pseudo db id 9 to denote SLRU pages, but maintains the current “raw”
data format of SLRU pages. The addition of page headers in our patch
resolves this issue [2] Munro mentions in this email [3].

Heikki Linnakangas then introduced patch on top of Munro’s patch that
modularizes the storage manager, allowing SLRUs to use it [4]. Instead
of using db id 9, SLRUs use spcOid 9, and each SLRU is its own relation.
Here, Heikki simplifies the storage manager by having each struct be
responsible for just one fork of a relation; thus increasing
extensibility of the smgr API, including for SLRUs. [5] We integrated
our changes introducing page headers for SLRU pages, and upgrade logic
to Heikki’s latest patch.

> > Rebased patch as per latest community changes since last email.

> This version doesn't actually build.
> https://cirrus-ci.com/task/4512310190931968
> [19:43:20.131] FAILED:
> src/test/modules/test_slru/test_slru.so.p/test_slru.c.o

As for the build failures, I was using make install to build, and make
check for regression tests. The build failures in this link are from
unit tests dependent on custom SLRUs, which would no longer apply. I’ve
attached another patch that removes these tests.

[1] https://www.postgresql.org/message
-id/CA%2BhUKGKCkbtOutcz5M8Z%3D0pgAkwdiR57Lxk7803rGsgiBNST6w%40mail.gmail
.com

[2] “I needed to disable checksums and in-page LSNs, since SLRU pages
hold raw data with no header. We'd probably eventually want regular
(standard? formatted?) pages (the real work here may be implementing
FPI for SLRUs so that checksums don't break your database on torn
writes). ”

[3] https://www.postgresql.org/message-id/CA%2BhUKGKAYze99B-jk9NoMp-
2BDqAgiRC4oJv%2BbFxghNgdieq8Q%40mail.gmail.com

[4] https://www.postgresql.org/message-id/flat/128709bc-992c-b57a-7174-
098433b7faa4%40iki.fi#a78d6250327795e95b02e9305e2d153e

[5] “I think things would be more clear if we
unbundled the forks at the SMGR level, so that we would have a separate
SMgrRelation struct for each fork. And let's rename it to SMgrFile to
make the role more clear. I think that would reduce the confusion when
we start using it for SLRUs; an SLRU is not a relation, after all. md.c
would still segment each logical file into 1 GB segments, but it would
not need to deal with forks.”

Attachment Content-Type Size
slru_to_buffer_cache_with_page_headers_v6.patch application/octet-stream 373.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-02-08 20:16:46 Re: Move defaults toward ICU in 16?
Previous Message Andres Freund 2023-02-08 20:02:35 Re: Logical replication timeout problem