Re: Draft for basic NUMA observability - Mailing list pgsql-hackers

On 4/5/25 11:37, Bertrand Drouvot wrote:
> Hi,
> 
> On Fri, Apr 04, 2025 at 09:25:57PM +0200, Tomas Vondra wrote:
>> OK,
>>
>> here's v25 after going through the patches once more, fixing the issues
>> mentioned by Bertrand, etc.
> 
> Thanks!
> 
>> I think 0001 and 0002 are fine,
> 
> Agree, I just have some cosmetic nits comments: please find them in
> nit-bertrand-0002.txt attached.
> 
>> I have a
>> couple minor questions about 0003.
>>
>> - I was wondering if maybe we should have some "global ID" of memory
>> page, so that with large memory pages it's indicated the buffers are on
>> the same memory page. Right now each buffer starts page_num from 0, but
>> it should not be very hard to have a global counter. Opinions?
> 
> I think that's a good idea. We could then add a new column (say os_page_id) that
> would help identify which buffers are sharing the same "physical" page.
> 

I was thinking we'd change the definition of the existing page_num
column, i.e. it wouldn't be 0..N sequence for each buffer, but a global
page ID. But I don't know if this would be useful in practice.

>> 0003
>> ----
>> - Minor formatting tweaks, comment improvements.
>> - Isn't this comment a bit confusing / misleading?
>>
>>     /* Get number of OS aligned pages */
>>
>> AFAICS the point is to adjust the allocated_size to be a multiple of
>> os-page_size, to get "all" memory pages the segment uses. But that's not
>> what I understand by "aligned page" (which is about there the page is
>> expected to start).
> 
> Agree, what about? 
> 
> "
> Align the start of the allocated size to an OS page size boundary and then get
> the total number of OS pages used by this segment"
> "
> 

Something like that. But I think it should be "align the size of ...",
we're not aligning the start.

>> - There's a comment at the end which talks about "ignored segments".
>> IMHO that type of information should be in the function comment,
>> but I'm
>> also not quite sure I understand what "output shared memory" is ...
> 
> I think that comes from the comments that are already in
> pg_get_shmem_allocations().
> 
> I think that those are located here and worded that way to ease to understand 
> what is not in with pg_get_shmem_allocations_numa() if one look at both
> functions. That said, I'm +1 to put this kind of comments in the function comment.
> 

OK. But I'm still not sure what "output shared memory" is about. Can you
explain what shmem segments are not included?


regards

-- 
Tomas Vondra