Re: Proposal to add page headers to SLRU pages

Given so many different approaches were discussed, I have started a wiki to record and collaborate all efforts towards SLRU improvements. The wiki provides a concise overview of all the ideas discussed and can serve as a portal for all historical discussions. Currently, the wiki summarizes four recent threads ranging from identifier format change to page header change, to moving SLRU into the main buffer pool, to reduce lock contention on SLRU latches. We can keep the patch related discussions in this thread and use the wiki as a live document for larger scale collaborations.

The wiki page is here: https://wiki.postgresql.org/wiki/SLRU_improvements

Regarding the benefits of this patch, here is a detailed explanation:

1. Checksum is added to each page, allowing us to verify if a page has been corrupted when read from the disk.
2. The ad-hoc LSN group structure is removed from the SLRU cache control data and is replaced by the page LSN in the page header. This allows us to use the same WAL protocol as used by pages in the main buffer pool: flush all redo logs up to the page LSN before flushing the page itself. If we move SLRU caches into the main buffer pool, this change fits naturally.
3. It leaves further optimizations open. We can continue to pursue the goal of moving SLRU into the main buffer pool, or we can follow the lock partition idea. This change by itself does not conflict with either proposal.

Also, the patch is now complete and is ready for review. All check-world tests including tap tests passed with this patch.

Regards,
Yong

From: Robert Haas <robertmhaas(at)gmail(dot)com>
Date: Friday, December 8, 2023 at 03:51
To: Debnath, Shawn <sdn(at)ebay(dot)com>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, Li, Yong <yoli(at)ebay(dot)com>, Shyrabokau, Anton <antons(at)ebay(dot)com>, Bagga, Rishu <bagrishu(at)amazon(dot)com>
Subject: Re: Proposal to add page headers to SLRU pages
External Email

On Thu, Dec 7, 2023 at 1:28 PM Debnath, Shawn <sdn(at)ebay(dot)com> wrote:
> What is being proposed now is the simple and core functionality of introducing
> page headers to SLRU pages while continuing to be in the SLRU cache. This
> allows the whole project to be iterative and reviewers to better reason about
> the smaller set of changes being introduced into the codebase.
>
> Once the set of on-disk changes are in, we can follow up on optimizations.
> It may be moving to buffer cache or reviewing Dilip's approach in [1], we
> will have the option to be flexible in our approach.

I basically agree with this. I don't think we should let the perfect
be the enemy of the good. Shooting down this patch because it doesn't
do everything that we want is a recipe for getting nothing done at
all.

That said, I don't think that the original post on this thread
provides a sufficiently clear and detailed motivation for making this
change. For this to eventually be committed, it's going to need (among
other things) a commit message that articulates a convincing rationale
for whatever changes it makes. Here's what the original email said:

> It adds a checksum to each SLRU page, tracks page LSN as if it is a standard page and eases future page enhancements.

Of those three things, in my opinion, the first is good and the other
two are too vague. I assume that most people who would be likely to
read a commit message would understand the value of pages having
checksums. But I can't immediately think of what the value of tracking
the page LSN as if it were a standard page might be, so that probably
needs more explanation. Similarly, at least one or two of the future
page enhancements that might be eased should be spelled out, and/or
the ways in which they would be made easier should be articulated.

--
Robert Haas
EDB: https://nam10.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.enterprisedb.com%2F&data=05%7C01%7Cyoli%40ebay.com%7C2cad2fe1de8a40f3167608dbf75de73c%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638375754901646398%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=hkccuGfpt1%2BKxuhk%2BJt%2F3HyYuJqQHYfizib76%2F9HtUU%3D&reserved=0<http://www.enterprisedb.com/>