Re: Draft for basic NUMA observability

From: Tomas Vondra <tomas(at)vondra(dot)me>
To: Andres Freund <andres(at)anarazel(dot)de>, Kirill Reshke <reshkekirill(at)gmail(dot)com>
Cc: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Draft for basic NUMA observability
Date: 2025-04-08 22:47:59
Message-ID: 91b09295-e2fe-4ca1-9db6-36dc87ead53e@vondra.me
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 4/8/25 16:59, Andres Freund wrote:
> Hi,
>
> On 2025-04-08 09:35:37 -0400, Andres Freund wrote:
>> On April 8, 2025 9:21:57 AM EDT, Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>>> On 4/8/25 15:06, Andres Freund wrote:
>>>> On 2025-04-08 17:44:19 +0500, Kirill Reshke wrote:
>>>> I think it's not right that something in src/port defines an SQL callable
>>>> function. The set of headers for that pull in a lot of things.
>>>>
>>>> Since the file relies on things like GUCs, I suspect this should be in
>>>> src/backend/port or such instead.
>>>>
>>>
>>> Yeah, I think you're right, src/backend/port seems like a better place
>>> for this. I'll look into moving that in the evening.
>>
>> On a second look I wonder if just the SQL function and perhaps the page size
>> function should be moved. There are FE programs that could potentially
>> benefit from num a awareness (e.g. pgbench).
>
> I would move pg_numa_available() to something like
> src/backend/storage/ipc/shmem.c.
>

Makes sense, done in the attached patch.

> I wonder if pg_numa_get_pagesize() should loose the _numa_ in the name, it's
> not actually directly NUMA related? If it were pg_get_pagesize() it'd fit in
> with shmem.c or such.
>

True. It's true it's not really "NUMA page", but page size for the shmem
segment. So renamed to pg_get_shmem_pagesize() and moved to shmem.c,
same as pg_numa_available().

The rest of pg_numa.c is moved to src/backend/port

> Or we could just make it work in FE code by making this part
>
> Assert(IsUnderPostmaster);
> Assert(huge_pages_status != HUGE_PAGES_UNKNOWN);
>
> if (huge_pages_status == HUGE_PAGES_ON)
> GetHugePageSize(&os_page_size, NULL);
>
> #ifndef FRONTEND - we don't currently support using huge pages in FE programs
> after all. But querying the page size might still be useful.
>

I don't really like this. Why shouldn't the FE program simply call
sysconf(_SC_PAGESIZE)? It'd be just confusing if in backend it'd also
verify huge page status.

>
> Regardless of all of that, I don't think the include of fmgr.h in pg_numa.h is
> needed?
>

Right, that was left there after moving the prototype into the .c file.

regards

--
Tomas Vondra

Attachment Content-Type Size
numa-fixes.patch text/x-patch 5.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2025-04-08 23:10:09 Re: Draft for basic NUMA observability
Previous Message Jan Wieck 2025-04-08 21:37:50 Re: Horribly slow pg_upgrade performance with many Large Objects