pgsql: Add support for basic NUMA awareness

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 ../pgsql/src/include/postgres.h:49,
from ../pgsql/src/port/pg_numa.c:16:
../pgsql/src/include/utils/elog.h:79:10: fatal error: utils/errcodes.h: No such file or directory
79 | #include "utils/errcodes.h"
| ^~~~~~~~~~~~~~~~~~
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 unlimited parallelism.

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 size function 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 size 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.

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 if 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?

I think part of the file would make sense to eventually use in FE code, we should move the SQL function and the page size 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 list if 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]https://github.com/technowledgy/postgresql-buildfarm-alpine/blob/main/Dockerfile. I'm installing
the package "ninja" there, which... doesn't even exist upstream [2]https://pkgs.alpinelinux.org/packages?name=*ninja*&amp;branch=v3.21&amp;repo=&amp;arch=x86_64&amp;origin=&amp;flagged=&amp;maintainer=.
Odd. There is ninja-build, though, which is at 1.12.1. [3]https://pkgs.alpinelinux.org/package/v3.21/community/x86_64/ninja-build

Turns out that the "ninja" I am installing is provided by samurai [4]https://pkgs.alpinelinux.org/package/v3.21/main/x86_64/samurai,
which seems to be a drop-in replacement [5]https://github.com/michaelforney/samurai 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
https://github.com/technowledgy/postgresql-buildfarm-alpine/blob/main/Dockerfile

[2]: https://pkgs.alpinelinux.org/packages?name=*ninja*&amp;branch=v3.21&amp;repo=&amp;arch=x86_64&amp;origin=&amp;flagged=&amp;maintainer=
https://pkgs.alpinelinux.org/packages?name=*ninja*&amp;branch=v3.21&amp;repo=&amp;arch=x86_64&amp;origin=&amp;flagged=&amp;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 list if 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

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