Adding NetBSD and OpenBSD to Postgres CI

On 01.11.24 10:17, Nazir Bilal Yavuz wrote:

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks
are not added to the upstream Postgres yet. The attached patch adds
NetBSD and OpenBSD tasks to the Postgres CI.

I made these tasks triggered manually like MinGW task to save CI credits
but a related line is commented out for now to trigger CFBot.

This seems useful to me. It would add some more testability for
LibreSSL for example.

I noticed that neither the existing FreeBSD task nor the new OpenBSD one
find the bsd_auth.h header. I thought this would be good to get more
testing of that code. Do you know why that is?

Hi,

Thanks for the patch!

On 2024-11-01 12:17:00 +0300, Nazir Bilal Yavuz wrote:

I made these tasks triggered manually like MinGW task to save CI credits
but a related line is commented out for now to trigger CFBot.

Oh, I need to pick my patch which allows repo-level config of which tasks run
back up. Then we can enable these by default for cfbot, but not for individual
repos...

+task:
+  depends_on: SanityCheck
+  # trigger_type: manual
+
+  env:
+    # Below are experimentally derived to be a decent choice.
+    CPUS: 2
+    BUILD_JOBS: 8
+    TEST_JOBS: 8
+
+    CIRRUS_WORKING_DIR: /home/postgres/postgres

Why do you need to set that?

+    CCACHE_DIR: /tmp/ccache_dir
+
+    PATH: /usr/sbin:$PATH
+
+    # Postgres interprets LANG as a 'en_US.UTF-8' but it is 'C', then

What does "Postgres interprets LANG as a 'en_US.UTF-8'" mean?

+    # Postgres tries to set 'LC_COLLATE' to 'en_US.UTF-8' but it is not
+    # changeable. Initdb fails because of that. So, LANG is forced to be 'C'.
+    LANG: "C"
+    LC_ALL: "C"

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_lib_dirs=/usr/pkg/lib -Dextra_include_dirs=/usr/pkg/include
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

Shouldn't something be added to PKG_CONFIG_PATH / --pkg-config-path?

For other OSs we have stanzas like

setup_additional_packages_script: |
#apt-get update
#DEBIAN_FRONTEND=noninteractive apt-get -y install ...

it'd be good to have something similar for openbsd/netbsd, given that most
won't be as familiar with that.

+      <<: *openbsd_task_template
+  sysinfo_script: |
+    locale
+    id
+    uname -a
+    ulimit -a -H && ulimit -a -S
+    env
+
+  ccache_cache:
+    folder: $CCACHE_DIR
+
+  create_user_script: |
+    useradd postgres
+    chown -R postgres:users /home/postgres
+    mkdir -p ${CCACHE_DIR}
+    chown -R postgres:users ${CCACHE_DIR}
+
+  # -Duuid=bsd is not set since 'bsd' uuid option
+  # is not working on NetBSD & OpenBSD. See
+  # https://www.postgresql.org/message-id/17358-89806e7420797025@postgresql.org
+  # And other uuid options are not available on NetBSD.
+  configure_script: |
+    su postgres <<-EOF
+      meson setup \
+        --buildtype debug \

I suspect it'd be good to either add -Og to cflags (as done in a bunch of
other tasks) or to use debugoptimized, given that the tests on these machines
are fairly slow.

+        -Dcassert=true -Dssl=openssl ${UUID} \
+        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
+        ${INCLUDE_DIRS} \
+        build
+    EOF

Should probably enable injection points.

+  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
+  upload_caches: ccache
+
+  test_world_script: |
+    su postgres <<-EOF
+      ulimit -c unlimited
+      # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
+      ulimit -p 256
+      meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+    EOF
+
+  on_failure:
+    <<: *on_failure_meson
+
+

Right now you don't seem to be collecting core files - but you're still
enabling them via ulimit -c unlimited. At least we shouldn't use ulimit -c
unlimited without collecting core files, but it'd probably be better to add
support for collecting core files. Shouldn't be too hard.

Greetings,

Andres Freund

On 1 Nov 2024, at 12:16, Peter Eisentraut <peter@eisentraut.org> wrote:

On 01.11.24 10:17, Nazir Bilal Yavuz wrote:

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks are not added to the upstream Postgres yet. The attached patch adds NetBSD and OpenBSD tasks to the Postgres CI.
I made these tasks triggered manually like MinGW task to save CI credits but a related line is commented out for now to trigger CFBot.

This seems useful to me. It would add some more testability for LibreSSL for example.

Seconded, I very much look forward to having LibreSSL in CI.

--
Daniel Gustafsson

Hi,

Thanks for the feedback!

On Fri, 1 Nov 2024 at 14:16, Peter Eisentraut <peter@eisentraut.org> wrote:

On 01.11.24 10:17, Nazir Bilal Yavuz wrote:

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks
are not added to the upstream Postgres yet. The attached patch adds
NetBSD and OpenBSD tasks to the Postgres CI.

I made these tasks triggered manually like MinGW task to save CI credits
but a related line is commented out for now to trigger CFBot.

This seems useful to me. It would add some more testability for
LibreSSL for example.

I noticed that neither the existing FreeBSD task nor the new OpenBSD one
find the bsd_auth.h header. I thought this would be good to get more
testing of that code. Do you know why that is?

The 'bsd_auth.h' file does not exist in FreeBSD.

For the OpenBSD, it exists but its compilation fails with:
'''
/usr/include/bsd_auth.h:93:1: error: unknown type name 'quad_t'
/usr/include/bsd_auth.h:94:1: error: unknown type name 'quad_t
'''

These 'quad_t' types are defined in the 'sys/types.h' file. If I apply
the changes below:
"""
 if cc.check_header('bsd_auth.h', required: bsd_authopt,
-    args: test_c_args, include_directories: postgres_inc)
+    args: test_c_args, prefix: '#include <sys/types.h>',
+    include_directories: postgres_inc)
   cdata.set('USE_BSD_AUTH', 1)
   bsd_auth = declare_dependency()
 endif
"""

Then, meson is able to find "bsd_auth.h" in the OpenBSD task and CI
finishes successfully [1]https://cirrus-ci.com/task/6516032758611968. But I am not sure if this is enough. Do we
need additional changes for the BSD authentication?

[1]: https://cirrus-ci.com/task/6516032758611968

--
Regards,
Nazir Bilal Yavuz
Microsoft

On 07.11.24 14:40, Nazir Bilal Yavuz wrote:

I noticed that neither the existing FreeBSD task nor the new OpenBSD one
find the bsd_auth.h header. I thought this would be good to get more
testing of that code. Do you know why that is?

The 'bsd_auth.h' file does not exist in FreeBSD.

For the OpenBSD, it exists but its compilation fails with:
'''
/usr/include/bsd_auth.h:93:1: error: unknown type name 'quad_t'
/usr/include/bsd_auth.h:94:1: error: unknown type name 'quad_t
'''

These 'quad_t' types are defined in the 'sys/types.h' file.

Makes sense. The synopsis in https://man.openbsd.org/authenticate.3 is:

#include <sys/types.h>
#include <login_cap.h>
#include <bsd_auth.h>

Hi,

On 2024-11-07 16:05:43 +0100, Peter Eisentraut wrote:

On 07.11.24 14:40, Nazir Bilal Yavuz wrote:

I noticed that neither the existing FreeBSD task nor the new OpenBSD one
find the bsd_auth.h header. I thought this would be good to get more
testing of that code. Do you know why that is?

The 'bsd_auth.h' file does not exist in FreeBSD.

For the OpenBSD, it exists but its compilation fails with:
'''
/usr/include/bsd_auth.h:93:1: error: unknown type name 'quad_t'
/usr/include/bsd_auth.h:94:1: error: unknown type name 'quad_t
'''

These 'quad_t' types are defined in the 'sys/types.h' file.

Makes sense. The synopsis in https://man.openbsd.org/authenticate.3 is:

#include <sys/types.h>
#include <login_cap.h>
#include <bsd_auth.h>

It doesn't really make sense to me - it seems like a bug if system headers
aren't standalone. And the synopsys including a header doesn't seem to
obviously indicate that every user has to do so.

But anyway, we probably just have to deal with the world as it, rather than
how it should be :(

Greetings,

Andres Freund

Hi,

On Fri, 1 Nov 2024 at 21:44, Andres Freund <andres@anarazel.de> wrote:

Hi,

Thanks for the patch!

On 2024-11-01 12:17:00 +0300, Nazir Bilal Yavuz wrote:

I made these tasks triggered manually like MinGW task to save CI credits
but a related line is commented out for now to trigger CFBot.

Oh, I need to pick my patch which allows repo-level config of which tasks run
back up. Then we can enable these by default for cfbot, but not for individual
repos...

+task:
+  depends_on: SanityCheck
+  # trigger_type: manual
+
+  env:
+    # Below are experimentally derived to be a decent choice.
+    CPUS: 2
+    BUILD_JOBS: 8
+    TEST_JOBS: 8
+
+    CIRRUS_WORKING_DIR: /home/postgres/postgres

Why do you need to set that?

Otherwise, it is set to /tmp path and the size of /tmp mount point is
1.2GB, which is not enough.

+    CCACHE_DIR: /tmp/ccache_dir
+
+    PATH: /usr/sbin:$PATH
+
+    # Postgres interprets LANG as a 'en_US.UTF-8' but it is 'C', then

What does "Postgres interprets LANG as a 'en_US.UTF-8'" mean?

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

+    # Postgres tries to set 'LC_COLLATE' to 'en_US.UTF-8' but it is not
+    # changeable. Initdb fails because of that. So, LANG is forced to be 'C'.
+    LANG: "C"
+    LC_ALL: "C"

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_lib_dirs=/usr/pkg/lib -Dextra_include_dirs=/usr/pkg/include
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

Shouldn't something be added to PKG_CONFIG_PATH / --pkg-config-path?

I don't think so. Both OSes are able to find pkgconfig at
'/usr/pkg/bin/pkg-config'. Am I missing something?

For other OSs we have stanzas like

setup_additional_packages_script: |
#apt-get update
#DEBIAN_FRONTEND=noninteractive apt-get -y install ...

it'd be good to have something similar for openbsd/netbsd, given that most
won't be as familiar with that.

Done.

+      <<: *openbsd_task_template
+  sysinfo_script: |
+    locale
+    id
+    uname -a
+    ulimit -a -H && ulimit -a -S
+    env
+
+  ccache_cache:
+    folder: $CCACHE_DIR
+
+  create_user_script: |
+    useradd postgres
+    chown -R postgres:users /home/postgres
+    mkdir -p ${CCACHE_DIR}
+    chown -R postgres:users ${CCACHE_DIR}
+
+  # -Duuid=bsd is not set since 'bsd' uuid option
+  # is not working on NetBSD & OpenBSD. See
+  # https://www.postgresql.org/message-id/17358-89806e7420797025@postgresql.org
+  # And other uuid options are not available on NetBSD.
+  configure_script: |
+    su postgres <<-EOF
+      meson setup \
+        --buildtype debug \

I suspect it'd be good to either add -Og to cflags (as done in a bunch of
other tasks) or to use debugoptimized, given that the tests on these machines
are fairly slow.

Done.

+        -Dcassert=true -Dssl=openssl ${UUID} \
+        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
+        ${INCLUDE_DIRS} \
+        build
+    EOF

Should probably enable injection points.

Done.

+  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
+  upload_caches: ccache
+
+  test_world_script: |
+    su postgres <<-EOF
+      ulimit -c unlimited
+      # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
+      ulimit -p 256
+      meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+    EOF
+
+  on_failure:
+    <<: *on_failure_meson
+
+

Right now you don't seem to be collecting core files - but you're still
enabling them via ulimit -c unlimited. At least we shouldn't use ulimit -c
unlimited without collecting core files, but it'd probably be better to add
support for collecting core files. Shouldn't be too hard.

Done. I separated this patch to make review easier.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Hi,

On 2024-11-12 11:38:11 +0300, Nazir Bilal Yavuz wrote:

On Fri, 1 Nov 2024 at 21:44, Andres Freund <andres@anarazel.de> wrote:

+    CCACHE_DIR: /tmp/ccache_dir
+
+    PATH: /usr/sbin:$PATH
+
+    # Postgres interprets LANG as a 'en_US.UTF-8' but it is 'C', then

What does "Postgres interprets LANG as a 'en_US.UTF-8'" mean?

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

Do you happen to have a reference to the failure? The environment variables +
the exact error message would be good. Kinda feels like that shouldn't
happen with a default netbsd install.

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_lib_dirs=/usr/pkg/lib -Dextra_include_dirs=/usr/pkg/include
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

Shouldn't something be added to PKG_CONFIG_PATH / --pkg-config-path?

I don't think so. Both OSes are able to find pkgconfig at
'/usr/pkg/bin/pkg-config'. Am I missing something?

--pkg-config-path is about the the path to pkg-config files, not the path to
the pkg-config binary. If set we shouldn't need the
extra_lib_dirs/extra_include_dirs, I think.

Right now you don't seem to be collecting core files - but you're still
enabling them via ulimit -c unlimited. At least we shouldn't use ulimit -c
unlimited without collecting core files, but it'd probably be better to add
support for collecting core files. Shouldn't be too hard.

Done. I separated this patch to make review easier.

+1

From cbea598b11e85b5c7090ca8e9cc05c35f0359f54 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Thu, 7 Nov 2024 15:48:31 +0300
Subject: [PATCH v2 1/3] Fix meson could not find bsd_auth.h

bsd_auth.h file needs to be compiled together with the 'sys/types.h' as
it has missing type definitions.

See synopsis at https://man.openbsd.org/authenticate.3
---
meson.build | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5b0510cef78..84107955d5d 100644
--- a/meson.build
+++ b/meson.build
@@ -551,7 +551,8 @@ test_c_args = cppflags + cflags
bsd_authopt = get_option('bsd_auth')
bsd_auth = not_found_dep
if cc.check_header('bsd_auth.h', required: bsd_authopt,
-    args: test_c_args, include_directories: postgres_inc)
+    args: test_c_args, prefix: '#include <sys/types.h>',
+    include_directories: postgres_inc)
cdata.set('USE_BSD_AUTH', 1)
bsd_auth = declare_dependency()
endif
--

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?

Which also confuses me some, at some point this presumably worked?

From cd5bb66a55e5226b543f5b7db9128cfa48e338e3 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Mon, 11 Nov 2024 13:23:22 +0300
Subject: [PATCH v2 2/3] Add NetBSD and OpenBSD tasks to the Postgres CI

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks
are not added to the upstream Postgres yet. This patch adds them.

Note: These tasks will be triggered manually to save CI credits but a
related line is commented out for now to trigger CFBot.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>

[1] https://github.com/anarazel/pg-vm-images
---
.cirrus.tasks.yml | 84 +++++++++++++++++++++++++++++++++++++++++++++++
.cirrus.yml | 10 ++++++
2 files changed, 94 insertions(+)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index fc413eb11ef..f338af902aa 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -213,6 +213,90 @@ task:
cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores

+task:
+  depends_on: SanityCheck
+  # trigger_type: manual
+
+  env:
+    # Below are experimentally derived to be a decent choice.
+    CPUS: 2

For cfbot it turns out to be easier to just use 4 cpus for everything. What
time difference do you get from 2 vs 4 CPUs?

+ BUILD_JOBS: 8
+ TEST_JOBS: 8

8 build/test jobs for 2 cpus sounds unlikely to be close to optimal. A bit
higher makes sense, but 4x?

+ CIRRUS_WORKING_DIR: /home/postgres/postgres

I'd add a comment explaining why we're setting this.

+ CCACHE_DIR: /tmp/ccache_dir

And is it ok to put the ccache dir here, given the limited size of /tmp?

+    PATH: /usr/sbin:$PATH
+
+  matrix:
+    - name: NetBSD - 10 - Meson

Given the image name doesn't include the version it seems we shouldn't include
it here either...

+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/pkg/include -Dextra_lib_dirs=/usr/pkg/lib
+        # initdb fails with: 'invalid locale settings' error on NetBSD.
+        # Force 'LANG' and 'LC_*' variables to be 'C'.
+        LANG: "C"
+        LC_ALL: "C"
+      setup_additional_packages_script: |
+        #pkgin -y install ...
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

So for netbsd we're not using any uuid support? Ah, I see, it's documented
further down. Maybe reference that, or move the comment around?

+  # -Duuid=bsd is not set since 'bsd' uuid option
+  # is not working on NetBSD & OpenBSD. See
+  # https://www.postgresql.org/message-id/17358-89806e7420797025@postgresql.org
+  # And other uuid options are not available on NetBSD.
+  configure_script: |
+    su postgres <<-EOF
+      meson setup \
+        --buildtype=debugoptimized \
+        -Dcassert=true -Dinjection_points=true \
+        -Dssl=openssl ${UUID} \
+        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
+        ${INCLUDE_DIRS} \
+        build
+    EOF

https://cirrus-ci.com/task/4879081273032704?logs=configure#L320

[14:48:50.729] Run-time dependency krb5-gssapi found: NO (tried pkgconfig and cmake)
[14:48:50.729] Library gssapi_krb5 found: NO

https://cirrus-ci.com/task/6286456156585984?logs=configure#L109

[14:49:28.049] Run-time dependency krb5-gssapi found: NO (tried pkgconfig and cmake)
[14:49:28.049] Library gssapi_krb5 found: NO

Is gss really not available / installed on either?

https://cirrus-ci.com/task/4879081273032704?logs=configure#L49-L51
[14:48:50.729] Run-time dependency tcl found: NO (tried pkgconfig and cmake)
[14:48:50.729] Library tcl found: NO
[14:48:50.729] Has header "tcl.h" with dependency -ltcl: NO

Is TCL not available on openbsd?

+  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
+  upload_caches: ccache
+
+  test_world_script: |
+    su postgres <<-EOF
+      # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
+      ulimit -p 256

Should we also increase the number of semaphores?

/messages/by-id/db2773a2-aca0-43d0-99c1-060efcd9954e@gmail.com

Perhaps it'd be better to update the system config earlier in the openbsd
specific portion of the test?

build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop || true
EOF
<<: *on_failure_meson
-    cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores
+    cores_script: src/tools/ci/cores_backtrace.sh bsd /tmp/cores

Hm, what's the deal with this change?

on_failure:
<<: *on_failure_meson
+    cores_script: |
+      # Although OSes are forced to core dump inside ${CORE_DUMP_DIR}, they may
+      # not obey this. So, move core files to the ${CORE_DUMP_DIR} directory.
+      find build/ -maxdepth 1 -type f -name '*.core' -exec mv '{}' ${CORE_DUMP_DIR} \;
+      src/tools/ci/cores_backtrace.sh ${OS_NAME} ${CORE_DUMP_DIR}

s/OSs are forced/we try to configure the OS/

Is -maxdepth 1 really sufficient? The tests run in subdirectories, don't they?

Greetings,

Andres Freund

Hi,

Missed something until just after I hit send:

On 2024-11-12 11:38:11 +0300, Nazir Bilal Yavuz wrote:

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'

...

+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'

Think these probably should be added to src/tools/ci/README

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2024-11-12 11:38:11 +0300, Nazir Bilal Yavuz wrote:

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

Do you happen to have a reference to the failure? The environment variables +
the exact error message would be good. Kinda feels like that shouldn't
happen with a default netbsd install.

On mamba's host (pretty vanilla NetBSD 10.0):

$ env | grep ^L
LOGNAME=tgl
$ LANG=C initdb
... works fine ...
$ rm -rf $PGDATA
$ locale -a | grep en_US
en_US.ISO8859-1
en_US.ISO8859-15
en_US.US-ASCII
en_US.UTF-8
$ LANG=en_US.UTF-8 initdb
The files belonging to this database system will be owned by user "tgl".
This user must also own the server process.

initdb: error: invalid locale settings; check LANG and LC_* environment variables

I seem to recall noticing this when I was setting up mamba, but
I didn't feel like pursuing it so I just set the animal to test
only C locale.

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?
Which also confuses me some, at some point this presumably worked?

The configure path does work on NetBSD 10, and has for some time.
I've never tested it with meson though.

regards, tom lane

I wrote:

Andres Freund <andres@anarazel.de> writes:

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?
Which also confuses me some, at some point this presumably worked?

The configure path does work on NetBSD 10, and has for some time.

Ah, sorry, I'd forgotten that bsd_auth.h only exists on OpenBSD.
We correctly detect that the header is not there on NetBSD.

I tried it on OpenBSD 7.0, which is the only OpenBSD image I have
laying about at the moment, and configure correctly finds that the
"AC_CHECK_HEADER(bsd_auth.h, [], ...)" test succeeds. So it looks
to me like we should not need the sys/types.h include --- unless
they broke it in some more-recent release?

(It looks like none of our OpenBSD BF animals are testing
--with-bsd-auth, which is surely bad.)

regards, tom lane

Hi,

On 2024-12-17 12:44:46 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2024-11-12 11:38:11 +0300, Nazir Bilal Yavuz wrote:

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

Do you happen to have a reference to the failure? The environment variables +
the exact error message would be good. Kinda feels like that shouldn't
happen with a default netbsd install.

On mamba's host (pretty vanilla NetBSD 10.0):

$ env | grep ^L
LOGNAME=tgl
$ LANG=C initdb
... works fine ...
$ rm -rf $PGDATA
$ locale -a | grep en_US
en_US.ISO8859-1
en_US.ISO8859-15
en_US.US-ASCII
en_US.UTF-8
$ LANG=en_US.UTF-8 initdb
The files belonging to this database system will be owned by user "tgl".
This user must also own the server process.

initdb: error: invalid locale settings; check LANG and LC_* environment variables

I seem to recall noticing this when I was setting up mamba, but
I didn't feel like pursuing it so I just set the animal to test
only C locale.

Heh, I guess that's good enough for CI then too :)

It'd be helpful if the error message ought to at least include the category
being tested, perhaps it's just one category failing or such?

Greetings,

Andres Freund

Hi,

On 2024-12-17 13:09:49 -0500, Tom Lane wrote:

I wrote:

Andres Freund <andres@anarazel.de> writes:

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?
Which also confuses me some, at some point this presumably worked?

The configure path does work on NetBSD 10, and has for some time.

Ah, sorry, I'd forgotten that bsd_auth.h only exists on OpenBSD.
We correctly detect that the header is not there on NetBSD.

I tried it on OpenBSD 7.0, which is the only OpenBSD image I have
laying about at the moment, and configure correctly finds that the
"AC_CHECK_HEADER(bsd_auth.h, [], ...)" test succeeds. So it looks
to me like we should not need the sys/types.h include --- unless
they broke it in some more-recent release?

It does look like it was recently broken:
/messages/by-id/CAN55FZ0czTmfnfF=WOHJUZ0iZRiMz6Yf3FSMbPh4=Z5a_TDjKw@mail.gmail.com
/usr/include/bsd_auth.h:93:1: error: unknown type name 'quad_t'

I'm pretty sure that it *did* work not too long ago with meson, because I had
tested the meson stuff on openbsd when I wrote it.

And as Peter noted:

The synopsis in https://man.openbsd.org/authenticate.3 is:

#include <sys/types.h>
#include <login_cap.h>
#include <bsd_auth.h>

Which does look a bit like they expect sys/types.h to be included first.

Looking at their git mirror, I do indeed not see anything that'd include
sys/types.h:
https://github.com/openbsd/src/blob/master/include/bsd_auth.h
first includes machine/_types.h, which I think maps to:
https://github.com/openbsd/src/blob/master/sys/arch/amd64/include/_types.h#L4

and then sys/cdefs.h:
https://github.com/openbsd/src/blob/master/sys/sys/cdefs.h
which I think in turn includes
https://github.com/openbsd/src/blob/master/sys/arch/amd64/include/cdefs.h

None of those define quad_t. But none seems to have changed in a relevant way
recently either.

I wonder if this is due to a newer compiler having different implicit includes
or something?

(It looks like none of our OpenBSD BF animals are testing
--with-bsd-auth, which is surely bad.)

Agreed.

Greetings,

Andres Freund

Hi,

On 2024-12-17 14:25:01 -0500, Andres Freund wrote:

On 2024-12-17 13:09:49 -0500, Tom Lane wrote:

I wrote:

Andres Freund <andres@anarazel.de> writes:

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?
Which also confuses me some, at some point this presumably worked?

The configure path does work on NetBSD 10, and has for some time.

Ah, sorry, I'd forgotten that bsd_auth.h only exists on OpenBSD.
We correctly detect that the header is not there on NetBSD.

I tried it on OpenBSD 7.0, which is the only OpenBSD image I have
laying about at the moment, and configure correctly finds that the
"AC_CHECK_HEADER(bsd_auth.h, [], ...)" test succeeds. So it looks
to me like we should not need the sys/types.h include --- unless
they broke it in some more-recent release?

It does look like it was recently broken:
/messages/by-id/CAN55FZ0czTmfnfF=WOHJUZ0iZRiMz6Yf3FSMbPh4=Z5a_TDjKw@mail.gmail.com
/usr/include/bsd_auth.h:93:1: error: unknown type name 'quad_t'

I'm pretty sure that it *did* work not too long ago with meson, because I had
tested the meson stuff on openbsd when I wrote it.

And as Peter noted:

The synopsis in https://man.openbsd.org/authenticate.3 is:

#include <sys/types.h>
#include <login_cap.h>
#include <bsd_auth.h>

Which does look a bit like they expect sys/types.h to be included first.

Gah, configure does pass - because AC_CHECK_HEADER(), if includes is not passed
in, first includes what's defined in AC_INCLUDES_DEFAULT.

https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Default-Includes.html#Default-Includes
Expand to include-directives if defined, otherwise to:

#include <stdio.h>
#ifdef HAVE_SYS_TYPES_H
# include <sys/types.h>
#endif
...

So maybe my memory is just faulty and the bsd_auth.h path never worked with
openbsd + meson. Anyway, it looks like Bilal's patch to just add this to meson
seems to suffice.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2024-12-17 12:44:46 -0500, Tom Lane wrote:

On mamba's host (pretty vanilla NetBSD 10.0):
initdb: error: invalid locale settings; check LANG and LC_* environment variables

It'd be helpful if the error message ought to at least include the category
being tested, perhaps it's just one category failing or such?

Yeah, I will poke into this a little harder to see what's going on.

(Good sleuthing on the bsd_auth.h question, BTW.)

regards, tom lane

I wrote:

Andres Freund <andres@anarazel.de> writes:

It'd be helpful if the error message ought to at least include the category
being tested, perhaps it's just one category failing or such?

Yeah, I will poke into this a little harder to see what's going on.

So after a little testing, setlocale(LC_COLLATE, ...) fails to set the
locale to (apparently) anything other than C, but it works for other
categories. The reason is explained by "man setlocale":

Currently, setlocale() returns NULL and fails to change the locale when
LC_COLLATE is modified independently of other values.

It seems that on current NetBSD you have to use setlocale(LC_ALL, ...)
if you want to set the collation category. That's quite annoying
from our perspective, but it's not something I'm excited about fixing.
Given other work going on, we might abandon all this logic soon
anyway.

regards, tom lane

Hi,

Thanks for the review!

On Tue, 17 Dec 2024 at 19:21, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2024-11-12 11:38:11 +0300, Nazir Bilal Yavuz wrote:

On Fri, 1 Nov 2024 at 21:44, Andres Freund <andres@anarazel.de> wrote:

+    CCACHE_DIR: /tmp/ccache_dir
+
+    PATH: /usr/sbin:$PATH
+
+    # Postgres interprets LANG as a 'en_US.UTF-8' but it is 'C', then

What does "Postgres interprets LANG as a 'en_US.UTF-8'" mean?

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

Do you happen to have a reference to the failure? The environment variables +
the exact error message would be good. Kinda feels like that shouldn't
happen with a default netbsd install.

This was already discussed upthread. Would you like more information?

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_lib_dirs=/usr/pkg/lib -Dextra_include_dirs=/usr/pkg/include
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

Shouldn't something be added to PKG_CONFIG_PATH / --pkg-config-path?

I don't think so. Both OSes are able to find pkgconfig at
'/usr/pkg/bin/pkg-config'. Am I missing something?

--pkg-config-path is about the the path to pkg-config files, not the path to
the pkg-config binary. If set we shouldn't need the
extra_lib_dirs/extra_include_dirs, I think.

Yes, my bad. Done.

Right now you don't seem to be collecting core files - but you're still
enabling them via ulimit -c unlimited. At least we shouldn't use ulimit -c
unlimited without collecting core files, but it'd probably be better to add
support for collecting core files. Shouldn't be too hard.

Done. I separated this patch to make review easier.

+1

From cbea598b11e85b5c7090ca8e9cc05c35f0359f54 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Thu, 7 Nov 2024 15:48:31 +0300
Subject: [PATCH v2 1/3] Fix meson could not find bsd_auth.h

bsd_auth.h file needs to be compiled together with the 'sys/types.h' as
it has missing type definitions.

See synopsis at https://man.openbsd.org/authenticate.3
---
meson.build | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5b0510cef78..84107955d5d 100644
--- a/meson.build
+++ b/meson.build
@@ -551,7 +551,8 @@ test_c_args = cppflags + cflags
bsd_authopt = get_option('bsd_auth')
bsd_auth = not_found_dep
if cc.check_header('bsd_auth.h', required: bsd_authopt,
-    args: test_c_args, include_directories: postgres_inc)
+    args: test_c_args, prefix: '#include <sys/types.h>',
+    include_directories: postgres_inc)
cdata.set('USE_BSD_AUTH', 1)
bsd_auth = declare_dependency()
endif
--

Was about to apply that, but then started to wonder if we don't need the same
for configure? And it sure looks like that has the same problem?

Which also confuses me some, at some point this presumably worked?

This too was discussed upthread. Please let me know if you need more
information.

From cd5bb66a55e5226b543f5b7db9128cfa48e338e3 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Mon, 11 Nov 2024 13:23:22 +0300
Subject: [PATCH v2 2/3] Add NetBSD and OpenBSD tasks to the Postgres CI

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks
are not added to the upstream Postgres yet. This patch adds them.

Note: These tasks will be triggered manually to save CI credits but a
related line is commented out for now to trigger CFBot.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>

[1] https://github.com/anarazel/pg-vm-images
---
.cirrus.tasks.yml | 84 +++++++++++++++++++++++++++++++++++++++++++++++
.cirrus.yml | 10 ++++++
2 files changed, 94 insertions(+)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index fc413eb11ef..f338af902aa 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -213,6 +213,90 @@ task:
cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores

+task:
+  depends_on: SanityCheck
+  # trigger_type: manual
+
+  env:
+    # Below are experimentally derived to be a decent choice.
+    CPUS: 2

For cfbot it turns out to be easier to just use 4 cpus for everything. What
time difference do you get from 2 vs 4 CPUs?

I shared the timings at the first email of the thread [1]postgr.es/m/CAN55FZ0GXrojT2yUTrST5McJk8UWmYxUX8b696XjL01B1pKsxg%40mail.gmail.com. Copying the
timings from there:

╔══════════════════════╦════════╦═════════╗
║ CI Run Tim ║ ║ ║
║ (Only Test Step) ║ NetBSD ║ OpenBSD ║
║ (in minutes:seconds) ║ ║ ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 2, TJ: 4 ║ 13:18 ║ 17:07 ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 2, TJ: 6 ║ 11:01 ║ 16:23 ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 2, TJ: 8 ║ 10:14 ║ 15:41 ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 4, TJ: 4 ║ 11:46 ║ 16:03 ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 4, TJ: 6 ║ 09:56 ║ 14:59 ║
╠══════════════════════╬════════╬═════════╣
║ CPU: 4, TJ: 8 ║ 10:02 ║ 15:09 ║
╚══════════════════════╩════════╩═════════╝

+ BUILD_JOBS: 8
+ TEST_JOBS: 8

8 build/test jobs for 2 cpus sounds unlikely to be close to optimal. A bit
higher makes sense, but 4x?

I updated them with CPUS: 4 and JOBS: 8.

+ CIRRUS_WORKING_DIR: /home/postgres/postgres

I'd add a comment explaining why we're setting this.

Done.

+ CCACHE_DIR: /tmp/ccache_dir

And is it ok to put the ccache dir here, given the limited size of /tmp?

Yes, you are right; moved it to /home/postgres/cache.

+    PATH: /usr/sbin:$PATH
+
+  matrix:
+    - name: NetBSD - 10 - Meson

Given the image name doesn't include the version it seems we shouldn't include
it here either...

Done.

+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/pkg/include -Dextra_lib_dirs=/usr/pkg/lib
+        # initdb fails with: 'invalid locale settings' error on NetBSD.
+        # Force 'LANG' and 'LC_*' variables to be 'C'.
+        LANG: "C"
+        LC_ALL: "C"
+      setup_additional_packages_script: |
+        #pkgin -y install ...
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs

So for netbsd we're not using any uuid support? Ah, I see, it's documented
further down. Maybe reference that, or move the comment around?

Done.

+  # -Duuid=bsd is not set since 'bsd' uuid option
+  # is not working on NetBSD & OpenBSD. See
+  # https://www.postgresql.org/message-id/17358-89806e7420797025@postgresql.org
+  # And other uuid options are not available on NetBSD.
+  configure_script: |
+    su postgres <<-EOF
+      meson setup \
+        --buildtype=debugoptimized \
+        -Dcassert=true -Dinjection_points=true \
+        -Dssl=openssl ${UUID} \
+        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
+        ${INCLUDE_DIRS} \
+        build
+    EOF

https://cirrus-ci.com/task/4879081273032704?logs=configure#L320

[14:48:50.729] Run-time dependency krb5-gssapi found: NO (tried pkgconfig and cmake)
[14:48:50.729] Library gssapi_krb5 found: NO

https://cirrus-ci.com/task/6286456156585984?logs=configure#L109

[14:49:28.049] Run-time dependency krb5-gssapi found: NO (tried pkgconfig and cmake)
[14:49:28.049] Library gssapi_krb5 found: NO

Is gss really not available / installed on either?

NetBSD is fixed [2]https://github.com/anarazel/pg-vm-images/pull/108 but we require MIT Kerberos [3]f7431bca8b which requires a
gssapi_ext.h file. OpenBSD does not have MIT Kerberos nor gssapi_ext.h
file [4]postgr.es/m/3598083.1680976022%40sss.pgh.pa.us.

https://cirrus-ci.com/task/4879081273032704?logs=configure#L49-L51
[14:48:50.729] Run-time dependency tcl found: NO (tried pkgconfig and cmake)
[14:48:50.729] Library tcl found: NO
[14:48:50.729] Has header "tcl.h" with dependency -ltcl: NO

Is TCL not available on openbsd?

Version option is added, it is working now.

+  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
+  upload_caches: ccache
+
+  test_world_script: |
+    su postgres <<-EOF
+      # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
+      ulimit -p 256

Should we also increase the number of semaphores?

/messages/by-id/db2773a2-aca0-43d0-99c1-060efcd9954e@gmail.com

It is already increased while creating the bsd images [5]https://github.com/anarazel/pg-vm-images/blob/af8757bd5ed3f4055809bffde28334a8547dfced/scripts/bsd/netbsd-prep-postgres.sh#L30C1-L32C56 [6]https://github.com/anarazel/pg-vm-images/blob/af8757bd5ed3f4055809bffde28334a8547dfced/scripts/bsd/openbsd-prep-postgres.sh#L50C1-L53C63.

Perhaps it'd be better to update the system config earlier in the openbsd
specific portion of the test?

'ulimit -p' needs to be run as a postgres user, but postgres user is
created after the OpenBSD specific portion. I can put an if statement
to make it only work for OpenBSD. Does that sound good?

build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop || true
EOF
<<: *on_failure_meson
-    cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores
+    cores_script: src/tools/ci/cores_backtrace.sh bsd /tmp/cores

Hm, what's the deal with this change?

Removed, forgot to revert it before sending the patch.

on_failure:
<<: *on_failure_meson
+    cores_script: |
+      # Although OSes are forced to core dump inside ${CORE_DUMP_DIR}, they may
+      # not obey this. So, move core files to the ${CORE_DUMP_DIR} directory.
+      find build/ -maxdepth 1 -type f -name '*.core' -exec mv '{}' ${CORE_DUMP_DIR} \;
+      src/tools/ci/cores_backtrace.sh ${OS_NAME} ${CORE_DUMP_DIR}

s/OSs are forced/we try to configure the OS/

Is -maxdepth 1 really sufficient? The tests run in subdirectories, don't they?

You are right, maxdepth is removed now.

+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'

...

+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'

Think these probably should be added to src/tools/ci/README

Done.

[1]: postgr.es/m/CAN55FZ0GXrojT2yUTrST5McJk8UWmYxUX8b696XjL01B1pKsxg%40mail.gmail.com
[2]: https://github.com/anarazel/pg-vm-images/pull/108
[3]: f7431bca8b
[4]: postgr.es/m/3598083.1680976022%40sss.pgh.pa.us
[5]: https://github.com/anarazel/pg-vm-images/blob/af8757bd5ed3f4055809bffde28334a8547dfced/scripts/bsd/netbsd-prep-postgres.sh#L30C1-L32C56
[6]: https://github.com/anarazel/pg-vm-images/blob/af8757bd5ed3f4055809bffde28334a8547dfced/scripts/bsd/openbsd-prep-postgres.sh#L50C1-L53C63

--
Regards,
Nazir Bilal Yavuz
Microsoft

Hi,

I finally pushed this. The meson fix backpatched to 16.

I did some very minor polishing, reordering the OS lists to stay alphabetical,
instead of adding netbsd/openbsd somewhere to the front of lists.

Thanks for the patches!

Obviously not your fault, but I do think it's pretty crazy that with the same
available resources, netbsd and openbsd take considerably longer than linux
and freebsd, which both do a lot more (linux tests 32 and 64 bit with ubsan
with autoconf and asan with meson, freebsd tests a bunch of debugging
options). I wonder what is going wrong. I suspect we might be waiting for
the filesystem a lot, according to cirrus-ci's CPU usage graph we're not CPU
bound during the test phase. Or maybe they just scale badly.

Any chance you're interested in rebasing and expanding
/messages/by-id/20240413021221.hg53rvqlvldqh57i@awork3.anarazel.de

It'd be nice if we could enable these tasks on cfbot, where we bring our own
compute, while leaving them on manual for everyone else.

Greetings,

Andres Freund

Hi,

On Wed, 12 Feb 2025 at 17:49, Andres Freund <andres@anarazel.de> wrote:

I finally pushed this. The meson fix backpatched to 16.

I did some very minor polishing, reordering the OS lists to stay alphabetical,
instead of adding netbsd/openbsd somewhere to the front of lists.

Thanks!

Obviously not your fault, but I do think it's pretty crazy that with the same
available resources, netbsd and openbsd take considerably longer than linux
and freebsd, which both do a lot more (linux tests 32 and 64 bit with ubsan
with autoconf and asan with meson, freebsd tests a bunch of debugging
options). I wonder what is going wrong. I suspect we might be waiting for
the filesystem a lot, according to cirrus-ci's CPU usage graph we're not CPU
bound during the test phase. Or maybe they just scale badly.

Yes, I could not find the reason why either.

Any chance you're interested in rebasing and expanding
/messages/by-id/20240413021221.hg53rvqlvldqh57i@awork3.anarazel.de

It'd be nice if we could enable these tasks on cfbot, where we bring our own
compute, while leaving them on manual for everyone else.

Sure, I will work on this. Thank you for mentioning it.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Fri, Feb 14, 2025 at 2:02 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

On Wed, 12 Feb 2025 at 17:49, Andres Freund <andres@anarazel.de> wrote:

Obviously not your fault, but I do think it's pretty crazy that with the same
available resources, netbsd and openbsd take considerably longer than linux
and freebsd, which both do a lot more (linux tests 32 and 64 bit with ubsan
with autoconf and asan with meson, freebsd tests a bunch of debugging
options). I wonder what is going wrong. I suspect we might be waiting for
the filesystem a lot, according to cirrus-ci's CPU usage graph we're not CPU
bound during the test phase. Or maybe they just scale badly.

Yes, I could not find the reason why either.

I don't know much about NetBSD and OpenBSD, but my guess would be that
its UFS is serialising on synchronous I/O for some file/directory
operations that we do a lot of. Perhaps something like commit
0265e5c1 could be ported to those guys too? I suspect that Windows
has the same problem, I just don't know how to script a RAM disk as
all the web instructions start with "open the blah GUI and click on
the ...". I think the Linux file systems are just much better at that
stuff due to journaling and deferring I/O, but these other systems
probably issue synchronous writes, possibly with drive cache flushes
in some cases that you might not be able to turn off*. As for macOS,
I don't know much except that it doesn't seem too interested in
flushing the write cache generally, and is currently our 2nd fastest
OS on CI anyway.

Maybe we could try this?

https://man.netbsd.org/mount_tmpfs.8
https://man.openbsd.org/mount_tmpfs.8

FreeBSD has the same thing too, but I haven't looked into how that
compares to mounting UFS on a RAM disk device as I did it. Linux has
a tmpfs too, but we know that it refuses O_DIRECT so I wouldn't
propose that, it's essential to test those code paths. OpenBSD is the
only system in our universe without O_DIRECT, and I don't know what
NetBSD's policy is on O_DIRECT on tmpfs and wouldn't worry about it
too much anyway. (This is like a Zen riddle: what does it mean to
skip the page cache if the filesystem is the page cache? Mu. The
Linux answer is: no, go away.)

Thanks for working on these! I'll have another go at my workaround
for the SysV IPC resource limits on those two OSes, which is relevant
to the I/O worker stuff.

*In my hobbyist OS hacking I had an on/off switch for that for
throwaway FreeBSD CI systems where you don't care about durability,
though I didn't go anywhere with it after the RAM disk did the trick.

On Tue, Feb 18, 2025 at 11:33 AM Thomas Munro <thomas.munro@gmail.com> wrote:

Maybe we could try this?

https://man.netbsd.org/mount_tmpfs.8
https://man.openbsd.org/mount_tmpfs.8

NetBSD's test_world: 10:30 -> 3:23
OpenBSD test_world: 15:45 - >9:10

I think NetBSD would finish around 2nd place if turned on by default
like that. The OpenBSD part needs more work though, see attached...

Hi,

On Fri, 28 Feb 2025 at 19:26, Thomas Munro <thomas.munro@gmail.com> wrote:

On Tue, Feb 18, 2025 at 11:33 AM Thomas Munro <thomas.munro@gmail.com> wrote:

Maybe we could try this?

https://man.netbsd.org/mount_tmpfs.8
https://man.openbsd.org/mount_tmpfs.8

NetBSD's test_world: 10:30 -> 3:23
OpenBSD test_world: 15:45 - >9:10

Thank you for working on this! These are very nice speedups.

I think NetBSD would finish around 2nd place if turned on by default
like that. The OpenBSD part needs more work though, see attached...

+    #umount /usr/obj  # unused 5G on /dev/sd0j
+    #swapon /dev/sd0j # fails with ENXIO... what am I missing?

I think I found the problem, sd0j's fstype is not a swap. It worked like that:

$ disklabel -E sd0
$ umount /usr/obj
$ disklabel -E sd0 # prompts are: m -> j -> \n -> \n -> swap -> w -> q
$ disklabel -E sd0
$ swapon /dev/sd0j # runs successfully

--
Regards,
Nazir Bilal Yavuz
Microsoft

On Sat, Mar 1, 2025 at 6:24 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

I think I found the problem, sd0j's fstype is not a swap. It worked like that:

$ disklabel -E sd0
$ umount /usr/obj
$ disklabel -E sd0 # prompts are: m -> j -> \n -> \n -> swap -> w -> q
$ disklabel -E sd0
$ swapon /dev/sd0j # runs successfully

Thanks! I just piped those characters in and it worked, shaving
another couple of minutes off. The script is probably now about as
short and sweet as it needs to be? And the times are pretty good.

https://cirrus-ci.com/build/5275349266726912

On Sun, Mar 2, 2025 at 1:37 AM Thomas Munro <thomas.munro@gmail.com> wrote:

On Sat, Mar 1, 2025 at 6:24 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

I think I found the problem, sd0j's fstype is not a swap. It worked like that:

$ disklabel -E sd0
$ umount /usr/obj
$ disklabel -E sd0 # prompts are: m -> j -> \n -> \n -> swap -> w -> q
$ disklabel -E sd0
$ swapon /dev/sd0j # runs successfully

Thanks! I just piped those characters in and it worked, shaving
another couple of minutes off. The script is probably now about as
short and sweet as it needs to be? And the times are pretty good.

https://cirrus-ci.com/build/5275349266726912

And pushed.

I propose to turn these on by default tomorrow, unless someone speaks
up with reasons why we shouldn't. One non-reason to hold off is a
one-off report[1]/messages/by-id/CAOYmi+kr-Eh7utEDZBFq8Mc9byLLRF3ZNc0TaHt78SKwB4T5KA@mail.gmail.com from Jacob about an unexplained failure of the oauth
tests on NetBSD, but I'd prefer to see it flap if it's going to so we
can collect clues and tackle it. Oauth test coverage is coming on in
the build farm slowly anyway, no reason to hide it from our first line
of defence...

[1]: /messages/by-id/CAOYmi+kr-Eh7utEDZBFq8Mc9byLLRF3ZNc0TaHt78SKwB4T5KA@mail.gmail.com

On Mon, Mar 3, 2025 at 2:36 PM Thomas Munro <thomas.munro@gmail.com> wrote:

One non-reason to hold off is a
one-off report[1] from Jacob about an unexplained failure of the oauth
tests on NetBSD, but I'd prefer to see it flap if it's going to so we
can collect clues and tackle it.

Agreed.

--Jacob

Hi,

On 2025-03-04 11:35:30 +1300, Thomas Munro wrote:

On Sun, Mar 2, 2025 at 1:37 AM Thomas Munro <thomas.munro@gmail.com> wrote:

On Sat, Mar 1, 2025 at 6:24 AM Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

I think I found the problem, sd0j's fstype is not a swap. It worked like that:

$ disklabel -E sd0
$ umount /usr/obj
$ disklabel -E sd0 # prompts are: m -> j -> \n -> \n -> swap -> w -> q
$ disklabel -E sd0
$ swapon /dev/sd0j # runs successfully

Thanks! I just piped those characters in and it worked, shaving
another couple of minutes off. The script is probably now about as
short and sweet as it needs to be? And the times are pretty good.

https://cirrus-ci.com/build/5275349266726912

And pushed.

I propose to turn these on by default tomorrow, unless someone speaks
up with reasons why we shouldn't. One non-reason to hold off is a
one-off report[1] from Jacob about an unexplained failure of the oauth
tests on NetBSD, but I'd prefer to see it flap if it's going to so we
can collect clues and tackle it. Oauth test coverage is coming on in
the build farm slowly anyway, no reason to hide it from our first line
of defence...

I'm worried about enabling them by default eating up CI credits even
faster. Not for cfbot, where we bring our own compute resources, but for folks
own repos.

I quite evidently lost track of that patch, but I think we should instead pick
/messages/by-id/20240413021221.hg53rvqlvldqh57i@awork3.anarazel.de back
up, with that we could enable these tasks by default for cfbot, without
enabling them for everyone.

Greetings,

Andres Freund

On Tue, Mar 4, 2025 at 11:39 AM Andres Freund <andres@anarazel.de> wrote:

On 2025-03-04 11:35:30 +1300, Thomas Munro wrote:

I propose to turn these on by default tomorrow, unless someone speaks
up with reasons why we shouldn't.

I'm worried about enabling them by default eating up CI credits even
faster. Not for cfbot, where we bring our own compute resources, but for folks
own repos.

It would be nice to know what the expected cost in Cirrus credits is
for each task, and the per-month limit for personal repos. mingw must
be *vastly* more expensive than these little guys (licence cost +
run time).

I quite evidently lost track of that patch, but I think we should instead pick
/messages/by-id/20240413021221.hg53rvqlvldqh57i@awork3.anarazel.de back
up, with that we could enable these tasks by default for cfbot, without
enabling them for everyone.

WFM in the meantime until we can turbo-boost the TAP tests. I will respond
over there...

Hi,

On 2025-02-12 09:49:35 -0500, Andres Freund wrote:

I finally pushed this.

One thing I noticed is that occasionally the tests fail on openbsd after
exceeding the system wide number of files, not in postgres, but causing meson
test to fail. I could either put an increase of kern.maxfiles in the openbsd
image generation, or in the CI task, any opinions?

The soft nofiles limit is also rather ridiculously low, at 128. I'd not be
surprised if that hurts test performance slightly. OTOH, I guess it's good to
have some CI coverage of a low setting.

Greetings,

Andres Freund

Hi,

meson version is upgraded 1.7.0 in NetBSD and that causes ecpg tests
to fail with [1]https://cirrus-ci.com/task/5126123371102208?logs=test_world#L144:

'Could not open file
/home/postgres/postgres/build/src/interfaces/ecpg/test/compat_informix/dec_test.c
for reading'

This is already fixed on b2bdb972c0, attached patch just adds
$MBUILD_TARGET to the NetBSD and OpenBSD CI tasks.

[1]: https://cirrus-ci.com/task/5126123371102208?logs=test_world#L144

--
Regards,
Nazir Bilal Yavuz
Microsoft

On 8 Apr 2025, at 13:40, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

This is already fixed on b2bdb972c0, attached patch just adds
$MBUILD_TARGET to the NetBSD and OpenBSD CI tasks.

After review and testing, I applied this to master.

--
Daniel Gustafsson

Hi,

The openbsd/netbsd CI tasks are occasionally failing due to reaching the
process limit. See e.g.
https://cirrus-ci.com/task/5559228405907456
+psql: error: connection to server on socket "/tmp/pg_regress-nQ1wk1/.s.PGSQL.40056" failed: could not fork new process for connection: Resource temporarily unavailable

This is due to the ulimit -p 256 in .cirrus.tasks.yml not being high enough.
The ulimit on openbsd couldn't trivially be increased with ulimit, because of
hitting the hard limit. Instead of increasing the limit in the CI script, the
CI image generation now increases the limits:
https://github.com/anarazel/pg-vm-images/pull/129

I plan to push this patch soon.

Greetings,

Andres Freund

On 2026-01-05 13:36:50 -0500, Andres Freund wrote:

The openbsd/netbsd CI tasks are occasionally failing due to reaching the
process limit. See e.g.
https://cirrus-ci.com/task/5559228405907456
+psql: error: connection to server on socket "/tmp/pg_regress-nQ1wk1/.s.PGSQL.40056" failed: could not fork new process for connection: Resource temporarily unavailable

This is due to the ulimit -p 256 in .cirrus.tasks.yml not being high enough.
The ulimit on openbsd couldn't trivially be increased with ulimit, because of
hitting the hard limit. Instead of increasing the limit in the CI script, the
CI image generation now increases the limits:
https://github.com/anarazel/pg-vm-images/pull/129

I plan to push this patch soon.

And done.