Re: Add os_page_num to pg_buffercache

Hi,

On Tue, Jul 08, 2025 at 02:47:34PM +0000, Mircea Cadariu wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: tested, passed
> Documentation: tested, passed
>
> Hi Bertrand,
>
> Just tried out your patch, nice work, thought to leave a review as well.

Thanks for looking at it!

> Patch applied successfully on top of commit a27893df4 in master.
> Ran the tests in pg_buffercache and they pass including the new ones.
>
> Running "pagesize" on my laptop returns 16384.
>
> test=# SELECT current_setting('block_size');
> current_setting
> -----------------
> 8192
> (1 row)
>
> Given the above, the results are as expected:
>
> test=# select * from pg_buffercache_os_pages;
> bufferid | os_page_num
> ----------+-------------
> 1 | 0
> 2 | 0
> 3 | 1
> 4 | 1
> 5 | 2
> 6 | 2

Cool.

> I have noticed that pg_buffercache_os_pages would be the 3rd function
> which follows the same high-level structure (others being pg_buffercache_pages
> and pg_buffercache_numa_pages). I am wondering if this would be let's say
> "strike three" - time to consider extracting out a high-level "skeleton" function,
> with a couple of slots which would then be provided by the 3 variants.

Yeah, I tried to avoid code duplication for the "os pages" related stuff in
v1. I can check if more can be done (outside of the "os pages" related stuff).

Might be done in a dedicated patch though, I mean I don't think that should be
a blocker for this one.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com