Re: Draft for basic NUMA observability

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tomas Vondra <tomas(at)vondra(dot)me>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Draft for basic NUMA observability
Date: 2025-04-09 15:14:32
Message-ID: pew2r5qbhhobhnvmo3fln4cu4x52q7jnmzvlb2hvrtzrauxome@slcpmivdz2nq
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2025-04-09 16:33:14 +0200, Tomas Vondra wrote:
> From e1f093d091610d70fba72b2848f25ff44899ea8e Mon Sep 17 00:00:00 2001
> From: Tomas Vondra <tomas(at)vondra(dot)me>
> Date: Tue, 8 Apr 2025 23:31:29 +0200
> Subject: [PATCH 1/2] Cleanup of pg_numa.c
>
> This moves/renames some of the functions defined in pg_numa.c:
>
> * pg_numa_get_pagesize() is renamed to pg_get_shmem_pagesize(), and
> moved to src/backend/storage/ipc/shmem.c. The new name better reflects
> that the page size is not related to NUMA, and it's specifically about
> the page size used for the main shared memory segment.
>
> * move pg_numa_available() to src/backend/storage/ipc/shmem.c, i.e. into
> the backend (which more appropriate for functions callable from SQL).
> While at it, improve the comment to explain what page size it returns.
>
> * remove unnecessary includes from src/port/pg_numa.c, adding
> unnecessary dependencies (src/port should be suitable for frontent).
> These were leftovers from earlier patch versions.

I don't think the include in src/port/pg_numa.c were leftover? Just the one in
pg_numa.h, right?

I'd mention that the includes of postgres.h/fmgr.h is what caused missing
build-time dependencies and via that failures on buildfarm member dogfish.

> diff --git a/src/port/pg_numa.c b/src/port/pg_numa.c
> index 5e2523cf798..63dff799436 100644
> --- a/src/port/pg_numa.c
> +++ b/src/port/pg_numa.c
> @@ -13,17 +13,14 @@
> *-------------------------------------------------------------------------
> */
>
> -#include "postgres.h"
> +#include "c.h"
> #include <unistd.h>
>
> #ifdef WIN32
> #include <windows.h>
> #endif

I think this may not be needed anymore, that was just there for
GetSystemInfo(), right? Conversely, I suspect it may now be needed in the new
location of pg_numa_get_pagesize()?

> From 201f8be652e9344dfa247b035a66e52025afa149 Mon Sep 17 00:00:00 2001
> From: Tomas Vondra <tomas(at)vondra(dot)me>
> Date: Wed, 9 Apr 2025 13:29:31 +0200
> Subject: [PATCH 2/2] ci: Check for missing dependencies in meson build
>
> Extends the meson build on Debian to also check for missing dependencies
> by executing
>
> ninja -t missingdeps
>
> right after the build. This highlights unindended dependencies.
>
> Reviewed-by: Andres Freund <andres(at)anarazel(dot)de>
> https://postgr.es/m/CALdSSPi5fj0a7UG7Fmw2cUD1uWuckU_e8dJ+6x-bJEokcSXzqA@mail.gmail.com

FWIW, while I'd prefer it as a meson.build visible test(), I think it's ok to
have it just in CI until we have that. I would however also add it to the
windows job, as that's the most "different" type of build / source of missed
dependencies that wouldn't show up on our development systems.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Maksim.Melnikov 2025-04-09 15:21:50 Re: sync_standbys_defined read/write race on startup
Previous Message Tom Lane 2025-04-09 15:11:14 Re: Build macOS shared modules as dylib rather than bundle