Thread: pgsql: Add support for basic NUMA awareness
Add support for basic NUMA awareness Add basic NUMA awareness routines, using a minimal src/port/pg_numa.c portability wrapper and an optional build dependency, enabled by --with-libnuma configure option. For now this is Linux-only, other platforms may be supported later. A built-in SQL function pg_numa_available() allows checking NUMA support, i.e. that the server was built/linked with the NUMA library. The main function introduced is pg_numa_query_pages(), which allows determining the NUMA node for individual memory pages. Internally the function uses move_pages(2) syscall, as it allows batching, and is more efficient than get_mempolicy(2). Author: Jakub Wartak <jakub.wartak@enterprisedb.com> Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tomas Vondra <tomas@vondra.me> Discussion: https://postgr.es/m/CAKZiRmxh6KWo0aqRqvmcoaX2jUxZYb4kGp3N%3Dq1w%2BDiH-696Xw%40mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/65c298f61fc70f2f960437c05649f71b862e2c48 Modified Files -------------- .cirrus.tasks.yml | 2 + configure | 187 ++++++++++++++++++++++++++++++++++++ configure.ac | 14 +++ doc/src/sgml/func.sgml | 13 +++ doc/src/sgml/installation.sgml | 22 +++++ meson.build | 23 +++++ meson_options.txt | 3 + src/Makefile.global.in | 6 +- src/backend/utils/misc/guc_tables.c | 2 +- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 4 + src/include/pg_config.h.in | 3 + src/include/port/pg_numa.h | 40 ++++++++ src/include/storage/pg_shmem.h | 1 + src/makefiles/meson.build | 3 + src/port/Makefile | 1 + src/port/meson.build | 1 + src/port/pg_numa.c | 120 +++++++++++++++++++++++ 18 files changed, 444 insertions(+), 3 deletions(-)
> On 7 Apr 2025, at 23:18, Tomas Vondra <tomas.vondra@postgresql.org> wrote: > > Add support for basic NUMA awareness dogfish is a little bit upset it seems. ninja: job failed: ccache gcc -Isrc/port/libpgport.a.p -Isrc/include -I../pgsql/src/include -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64-Wall -Winvalid-pch -O2 -g -fno-strict-aliasing -fwrapv -fexcess-precision=standard -D_GNU_SOURCE -Wmissing-prototypes-Wpointer-arith -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type-Wshadow=compatible-local -Wformat-security -Wdeclaration-after-statement -Wmissing-variable-declarations-Wno-format-truncation -Wno-stringop-truncation -fPIC -DFRONTEND -MD -MQ src/port/libpgport.a.p/pg_numa.c.o-MF src/port/libpgport.a.p/pg_numa.c.o.d -o src/port/libpgport.a.p/pg_numa.c.o -c ../pgsql/src/port/pg_numa.c In file included from [01m[K../pgsql/src/include/postgres.h:49[m[K, from [01m[K../pgsql/src/port/pg_numa.c:16[m[K: [01m[K../pgsql/src/include/utils/elog.h:79:10:[m[K [01;31m[Kfatal error: [m[Kutils/errcodes.h: No such file or directory 79 | #include [01;31m[K"utils/errcodes.h"[m[K | [01;31m[K^~~~~~~~~~~~~~~~~~[m[K compilation terminated. Looking at pg_numa.c, shouldn't the include of postgres.h have an #ifndef FRONTEND guard for postgres_fe.h as well? -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 7 Apr 2025, at 23:18, Tomas Vondra <tomas.vondra@postgresql.org> wrote:
>> Add support for basic NUMA awareness
> dogfish is a little bit upset it seems.
Hm, but why isn't anything else?
I noticed this a little further up in dogfish's log:
ninja: bad depfile: expected ':', saw 's'
I wonder if there's something a bit wrong with dogfish's meson
and/or ninja versions.
> Looking at pg_numa.c, shouldn't the include of postgres.h have an #ifndef
> FRONTEND guard for postgres_fe.h as well?
No, because it's evidently backend-only code. The real question
is why is it in src/port/, because everything there gets built
for both frontend and backend.
regards, tom lane
Hi, On April 8, 2025 10:05:48 AM EDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Daniel Gustafsson <daniel@yesql.se> writes: >>> On 7 Apr 2025, at 23:18, Tomas Vondra <tomas.vondra@postgresql.org> wrote: >>> Add support for basic NUMA awareness > >> dogfish is a little bit upset it seems. > >Hm, but why isn't anything else? I assume it's a question of degree of parallelism. I was able to repro the failure when building from a clean tree with unlimitedparallelism. >I noticed this a little further up in dogfish's log: > >ninja: bad depfile: expected ':', saw 's' I noticed that too - but it happened even before the hard failures. Will look at it in a bit. >> Looking at pg_numa.c, shouldn't the include of postgres.h have an #ifndef >> FRONTEND guard for postgres_fe.h as well? > >No, because it's evidently backend-only code. The real question >is why is it in src/port/, because everything there gets built >for both frontend and backend. I think part of the file would make sense to eventually use in FE code, we should move the SQL function and the page sizefunction out. Greetings, Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund <andres@anarazel.de> writes:
> On April 8, 2025 10:05:48 AM EDT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I noticed this a little further up in dogfish's log:
>>
>> ninja: bad depfile: expected ':', saw 's'
> I noticed that too - but it happened even before the hard failures. Will look at it in a bit.
After catching up on the hackers thread, I wondered if this isn't
actually part of the identical issue, that is ninja is noticing
that there's not a dependency leading to that include file.
> I think part of the file would make sense to eventually use in FE code, we should move the SQL function and the page
sizefunction out.
Works for me. But a file in src/port/ should really use only c.h
if at all possible. Otherwise (as Andrew said) it has to choose
one of postgres.h and postgres_fe.h depending on FRONTEND.
regards, tom lane
Hi, Walther, adding you because of the failure on buildfarm animal dogfish. The main issue is something unrelated to your animal (except it was the only one to catch it), but there's one oddity: On 2025-04-08 10:36:32 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On April 8, 2025 10:05:48 AM EDT, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> I noticed this a little further up in dogfish's log: > >> > >> ninja: bad depfile: expected ':', saw 's' > > > I noticed that too - but it happened even before the hard failures. Will look at it in a bit. > > After catching up on the hackers thread, I wondered if this isn't > actually part of the identical issue, that is ninja is noticing > that there's not a dependency leading to that include file. No, it seems to be something else - that "error" is happening even in the backbranches, which have been building without an issue for 167 days. But I'm rather bewildered - I can't actually build with the professed version of ninja, it errors out: ninja-1.9.0 src/bin/psql/sql_help.c ninja: error: build.ninja:8462: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing list ifit affects you So this must be some version of ninja between 1.9 and 1.10 (where the multiple outputs support was added). What's particularly weird about that is that the rest of the distro seems much newer. gcc 14.2.1, meson 1.6.1, LLVM 18. What's a < 2020 ninja doing with those surroundings? > > I think part of the file would make sense to eventually use in FE code, we should move the SQL function and the pagesize function out. > > Works for me. But a file in src/port/ should really use only c.h > if at all possible. Otherwise (as Andrew said) it has to choose > one of postgres.h and postgres_fe.h depending on FRONTEND. Yea. I think after moving the rest c.h should suffice. Greetings, Andres Freund
Andres Freund: > But I'm rather bewildered - I can't actually build with the professed version > of ninja, it errors out: > > ninja-1.9.0 src/bin/psql/sql_help.c > ninja: error: build.ninja:8462: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing listif it affects you > > So this must be some version of ninja between 1.9 and 1.10 (where the multiple > outputs support was added). > > What's particularly weird about that is that the rest of the distro seems much > newer. gcc 14.2.1, meson 1.6.1, LLVM 18. What's a < 2020 ninja doing with > those surroundings? The Dockerfile for the image running dogfish is at [1]. I'm installing the package "ninja" there, which... doesn't even exist upstream [2]. Odd. There is ninja-build, though, which is at 1.12.1. [3] Turns out that the "ninja" I am installing is provided by samurai [4], which seems to be a drop-in replacement [5] for ninja: > samurai implements the ninja build language through version 1.9.0 except for MSVC dependency handling (deps = msvc). It uses the same format for .ninja_log and .ninja_deps as ninja, currently version 5 and 4 respectively. I have not followed the remainder of the thread.. would you rather have me try to switch to the real ninja, or should we keep samurai for some variation in the buildfarm? Best, Wolfgang [1]: https://github.com/technowledgy/postgresql-buildfarm-alpine/blob/main/Dockerfile [2]: https://pkgs.alpinelinux.org/packages?name=*ninja*&branch=v3.21&repo=&arch=x86_64&origin=&flagged=&maintainer= [3]: https://pkgs.alpinelinux.org/package/v3.21/community/x86_64/ninja-build [4]: https://pkgs.alpinelinux.org/package/v3.21/main/x86_64/samurai [5]: https://github.com/michaelforney/samurai
Hi, On 2025-04-08 17:17:21 +0200, Wolfgang Walther wrote: > Andres Freund: > > But I'm rather bewildered - I can't actually build with the professed version > > of ninja, it errors out: > > > > ninja-1.9.0 src/bin/psql/sql_help.c > > ninja: error: build.ninja:8462: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing listif it affects you > > > > So this must be some version of ninja between 1.9 and 1.10 (where the multiple > > outputs support was added). > > > > What's particularly weird about that is that the rest of the distro seems much > > newer. gcc 14.2.1, meson 1.6.1, LLVM 18. What's a < 2020 ninja doing with > > those surroundings? > > The Dockerfile for the image running dogfish is at [1]. I'm installing the > package "ninja" there, which... doesn't even exist upstream [2]. Odd. There > is ninja-build, though, which is at 1.12.1. [3] > > Turns out that the "ninja" I am installing is provided by samurai [4], which > seems to be a drop-in replacement [5] for ninja: > > > samurai implements the ninja build language through version 1.9.0 except > for MSVC dependency handling (deps = msvc). It uses the same format for > .ninja_log and .ninja_deps as ninja, currently version 5 and 4 respectively. > > I have not followed the remainder of the thread.. would you rather have me > try to switch to the real ninja, or should we keep samurai for some > variation in the buildfarm? Ah, that explains that. Since it seems to be working, that one warning aside, I don't mind if it continues running with it. But it'd be nice to add a note to the buildfarm animal indicating it's using samurai instead of ninja (setnotes.pl). Greetings, Andres Freund
Andres Freund: > But it'd be nice to add a note > to the buildfarm animal indicating it's using samurai instead of ninja > (setnotes.pl). Did that! Best, Wolfgang
> On 7 Apr 2025, at 23:18, Tomas Vondra <tomas.vondra@postgresql.org> wrote: > > Add support for basic NUMA awareness It seems like this commit didn't run autoheader, which leaves a trivial diff in pg_config.h.in carried over for future callers. It doesn't change anuything really as the HAVE_LIBNUMA macro isn't used, but for completeness sake we should probably add it? -- Daniel Gustafsson
Attachment
On Wed, Apr 16, 2025 at 9:30 AM Daniel Gustafsson <daniel@yesql.se> wrote: > It seems like this commit didn't run autoheader, which leaves a trivial diff in > pg_config.h.in carried over for future callers. It doesn't change anuything > really as the HAVE_LIBNUMA macro isn't used, but for completeness sake we > should probably add it? +1, I ran autoreconf during a rebase today and had to undo the new NUMA hunks :D Your patch looks the same as what I'm carrying locally. Thanks, --Jacob
On 4/16/25 18:34, Jacob Champion wrote: > On Wed, Apr 16, 2025 at 9:30 AM Daniel Gustafsson <daniel@yesql.se> wrote: >> It seems like this commit didn't run autoheader, which leaves a trivial diff in >> pg_config.h.in carried over for future callers. It doesn't change anuything >> really as the HAVE_LIBNUMA macro isn't used, but for completeness sake we >> should probably add it? > > +1, I ran autoreconf during a rebase today and had to undo the new NUMA hunks :D > > Your patch looks the same as what I'm carrying locally. > Ah, I didn't notice the autoheader thing. The patch looks fine to me. Daniel, do you intend to push it, or do you want me to do that? regards -- Tomas Vondra
> On 16 Apr 2025, at 20:15, Tomas Vondra <tomas@vondra.me> wrote: > > On 4/16/25 18:34, Jacob Champion wrote: >> On Wed, Apr 16, 2025 at 9:30 AM Daniel Gustafsson <daniel@yesql.se> wrote: >>> It seems like this commit didn't run autoheader, which leaves a trivial diff in >>> pg_config.h.in carried over for future callers. It doesn't change anuything >>> really as the HAVE_LIBNUMA macro isn't used, but for completeness sake we >>> should probably add it? >> >> +1, I ran autoreconf during a rebase today and had to undo the new NUMA hunks :D >> >> Your patch looks the same as what I'm carrying locally. > > Ah, I didn't notice the autoheader thing. The patch looks fine to me. > Daniel, do you intend to push it, or do you want me to do that? I was just in the terminal pushing it, thanks for confirming the patch was good. -- Daniel Gustafsson