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