Hi, On 2025-03-05 12:29:15 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > I guess we could be add a "standardized" section at the top of each task > > describing their oddities? Not sure it's worth it. > > I think this does need to be documented somewhere/somehow, just so > that people don't waste time focusing on "it's failing on FreeBSD" > when the actual cause is some other thing we happened to load > onto that task.
0002 is a first draft for one way to do this. Of course this still requires somebody analyzing a failure to look at cirrus.tasks.yml, but I don't know if we can do a whole lot about that? I wondered about making the SPECIAL thing an environment variable instead of a comment, that way it'd probably be visible to cfbot. But I don't really see what it could do with that information. I guess we could make the SPECIAL: comments into echos in a special_script: that way it would show up as an explicit "section" in the per-task CI output. But I don't know how much that would help, the failures due to the tasks specialness could be during build, testing etc, so the "special" section won't necessarily be close to the failing step. Greetings, Andres Freund
>From 5e75ce6daeb293facbbcff0637d48b87ba40e592 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 5 Mar 2025 12:08:58 -0500 Subject: [PATCH v2 1/2] ci: freebsd: Specify debug_parallel_query=regress A lot of buildfarm animals run with debug_parallel_query=regress, while CI didn't test that. That lead to the annoying situation of only noticing related test instabilities after merging changes upstream. FreeBSD was chosen because it's a relatively fast task. It also tests debug_write_read_parse_plan_trees etc, which probably is exercised a bit more heavily with debug_parallel_query=regress. ci-os-only: freebsd Discussion: https://postgr.es/m/zbuk4mlov22yfoktf5ub3lwjw2b7ezwphwolbplthepda42int@h6wpvq7orc44 --- .cirrus.tasks.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index e7482da1fdd..a4cd0c76e80 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -139,7 +139,14 @@ task: CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS CFLAGS: -Og -ggdb - PG_TEST_INITDB_EXTRA_OPTS: -c debug_copy_parse_plan_trees=on -c debug_write_read_parse_plan_trees=on -c debug_raw_expression_coverage_test=on + # Several buildfarm animals enable these options. Without testing them + # during CI, it would be easy to cause breakage on the buildfarm with CI + # passing. + PG_TEST_INITDB_EXTRA_OPTS: >- + -c debug_copy_parse_plan_trees=on + -c debug_write_read_parse_plan_trees=on + -c debug_raw_expression_coverage_test=on + -c debug_parallel_query=regress PG_TEST_PG_UPGRADE_MODE: --link <<: *freebsd_task_template -- 2.48.1.76.g4e746b1a31.dirty
>From 923f7766bcc80f6bc094271b7073ec76a07e1e27 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 5 Mar 2025 12:50:05 -0500 Subject: [PATCH v2 2/2] ci: Document what makes certain tasks special To increase coverage without drastically increasing CI resource usage, we have different CI tasks test different things (e.g. the linux tasks use sanitizers). Unfortunately that can create confusing situations where CI fails on some OS, but not others, without the problem appearing to be platform dependent. To, partially, address that, add a comment, prefixed with SPECIAL, to each task that we use to test in some non-default way. Discussion: https://postgr.es/m/321570.1741195...@sss.pgh.pa.us --- .cirrus.tasks.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index a4cd0c76e80..57469518d86 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -2,6 +2,12 @@ # # For instructions on how to enable the CI integration in a repository and # further details, see src/tools/ci/README +# +# +# NB: Different tasks intentionally test with different, non-default, +# configurations, to increase the chance of catching problems. Each task with +# non-obvious non-default documents their oddity at the top of the task, +# prefixed by "SPECIAL:". env: @@ -55,6 +61,10 @@ on_failure_meson: &on_failure_meson # To avoid unnecessarily spinning up a lot of VMs / containers for entirely # broken commits, have a minimal task that all others depend on. +# +# SPECIAL: +# - Builds with --auto-features=disabled and thus almost no enabled +# dependencies. task: name: SanityCheck @@ -125,6 +135,11 @@ task: src/tools/ci/cores_backtrace.sh linux /tmp/cores +# SPECIAL: +# - Uses postgres specific CPPFLAGS that increase test coverage +# - Specifies configuration options that test reading/writing/copying of node trees +# - Specifies debug_parallel_query=regress, to catch related issues during CI +# - Also runs tests against a running postgres instance, see test_running_script task: name: FreeBSD - Meson @@ -355,6 +370,7 @@ LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >- -Duuid=e2fs +# Check SPECIAL in the matrix: below task: env: CPUS: 4 @@ -435,6 +451,10 @@ task: libcurl4-openssl-dev:i386 \ matrix: + # SPECIAL: + # - Uses address sanitizer, sanitizer failures are typically printed in + # the server log + # - Configures postgres with a small segment size - name: Linux - Debian Bookworm - Autoconf env: @@ -470,6 +490,10 @@ task: on_failure: <<: *on_failure_ac + # SPECIAL: + # - Uses undefined behaviour and alignment sanitizers, sanitizer failures + # are typically printed in the server log + # - Test both 64bit and 32 bit builds - name: Linux - Debian Bookworm - Meson env: @@ -530,6 +554,11 @@ task: cores_script: src/tools/ci/cores_backtrace.sh linux /tmp/cores +# NB: macOS is by far the most expensive OS to run CI for, therefore no +# expensive additional checks should be added. +# +# SPECIAL: +# - Enables --clone for pg_upgrade and pg_combinebackup task: name: macOS - Sonoma - Meson -- 2.48.1.76.g4e746b1a31.dirty
