| From: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Greg Sabino Mullane <htamfids(at)gmail(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
| Subject: | Re: CI slowdown due to PG_TEST_INITDB_EXTRA_OPTS |
| Date: | 2026-09-07 11:06:18 |
| Message-ID: | CAN55FZ3V+pUkzB4Jc9Avw69m96qFcY_amQP1RoCu3RYLM++zKw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Sun, 6 Sept 2026 at 21:12, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
> On Thu, Aug 13, 2026, Greg Sabino Mullane wrote:
> > But they can still use PG_TEST_INITDB_EXTRA_OPTS, right? We are not
> > replacing the var, just adding a new one, I thought.
>
> Yes. I read Nazir's v2. It keeps PG_TEST_INITDB_EXTRA_OPTS
> available for actual initdb options. Both Cluster.pm and pg_regress
> handle the new variable consistently, and the CI changes move only
> server GUCs to it. The patch LGTM.
Thank you for looking into this!
>
> The lowest-hanging fruit I found is splitting the Linux Meson 64-bit
> ASAN tests across two runners, as we already do for the Windows Visual
> Studio job. This does not reduce the total work and actually
> duplicates the configure and build steps, but it does reduce how long
> we wait for CI.
>
> In one run, the two Test world slices took 8:33 and 12:08. Two
> unsliced jobs running at the same time took 23:45 and 25:05. Together
> the slices ran the same 412 tests. In a complete run, the gain is
> naturally limited by the next-slowest job.
>
> The CI image currently has Meson 1.7, so the patch emulates Meson 1.8's
> round-robin --slice using the test list.
First question that comes to mind is why we are doing this only for
Linux 64 Meson and not for all jobs. I think there are two answers:
1. We can't do this for Linux Autoconf as far as I know, so it doesn't
make sense for tasks that take less or similar time.
2. The free concurrency limit for GitHub Actions is 20, and our main
CI tasks use 8 (9 with your patch). It would be best to not exceed 10
so that we can run two CI tasks simultaneously.
So, I think this change makes sense. Also we can apply the same change
to the next longest running task, which is MinGW 64 depending on my
testing.
My review:
+ readarray -t test_names < <(
+ meson test ${{env.MTEST_ARGS}} ${{env.MTEST_TARGET}} \
+ --list --no-suite setup |
+ awk -F ' - ' \
+ -v slice=${{ matrix.slice}} \
+ -v slices=${{ matrix.num_slices}} \
+ '(NR - 1) % slices == slice - 1 {
+ name = $NF
+ sub(/:[^ ]+ \/ /, ":", name)
+ print name
+ }'
+ )
This is complicated but I don't have a better solution. One point is
that 'meson test --list' doesn't guarantee the order of the returned
list, so we might miss tests if the order differs between tasks. I
recommend sorting the list names before slicing.
- - *upload_logs_step
+ - name: Upload logs
+ if: failure() && !cancelled()
+ uses: actions/upload-artifact(at)v7
+ with:
+ name: logs-${{ github.job }}-${{ matrix.slice }}-${{
github.run_id }}-${{ github.run_attempt }}
+ path: |
+ **/*.log
+ **/*.diffs
+ **/regress_log_*
+ **/crashlog-*.txt
+ build/meson-logs/**
+ **/config.log
+ if-no-files-found: ignore
We have the same problem for the Windows VS job; otherwise, two
artifacts could end up having the same name. I remember sending a
patch to fix this, but I don't recall where it is. I think we can save
this step as something like 'upload_logs_step_slice' and use it in the
Windows VS task as well.
--
Regards,
Nazir Bilal Yavuz
Microsoft
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2026-09-07 11:08:03 | Re: Offline data checksum changes can cause incorrect checksum state on standbys |
| Previous Message | Ilia Evdokimov | 2026-09-07 11:04:14 | Re: COALESCE patch |