From 3173cd26fbaae668c8445307d56b94154351684e Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 00:40:36 -0600 Subject: [PATCH 1/8] ci: move the Spark 4.1 and Iceberg 1.11 suites behind the merge queue Leave the Linux build as the whole PR tier. Spark 4.1 (catalyst and sql_core) and Iceberg 1.11 were the last test suites still running on every push to every pull request, and with agent-driven review and agent-driven replies to review a pull request now goes through several more push rounds before it is queued. The queue runs both suites against the merge result before anything lands regardless. `run-spark-4.1-tests` now gates the whole Spark 4.1 call, so it appears on both of that job's POLICY outputs; `run-spark-4.1-hive-tests` keeps selecting only the hive shards. `run-iceberg-tests` opts a pull request into all four Iceberg versions. Pin the new routing in check-ci-config.py and update the workflows README and the contributor guide. Closes #5938 --- .github/workflows/README.md | 78 +++++++++++-------- .github/workflows/ci.yml | 31 ++++---- dev/ci/check-ci-config.py | 38 +++++++-- dev/ci/compute-changes.py | 29 ++++--- dev/ci/spark-sql-modules.py | 4 +- docs/source/contributor-guide/ci.md | 34 ++++---- .../contributor-guide/iceberg-spark-tests.md | 13 ++-- .../contributor-guide/spark-sql-tests.md | 5 +- 8 files changed, 144 insertions(+), 88 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 71dfbd6f19..19d479fe33 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -15,15 +15,20 @@ Merging goes through GitHub's merge queue, configured by the `Merge Queue` ruleset in `.asf.yaml`. That splits CI into two tiers: - **PR tier** (`pr`): fast feedback while a change is being iterated on. - The Linux build, Spark 4.1 (catalyst and `sql_core` only) and Iceberg 1.11. -- **Queue tier** (`queue`): the authoritative gate. Everything the PR tier - runs, plus the macOS build, the benchmark compile check, the Delta contrib - build gate, the PyArrow UDF suite, the Spark 4.1 `sql_hive` shards, Spark - 3.5/4.0 and Iceberg 1.8/1.9/1.10, evaluated against the merge result - rather than against the PR head. + The Linux build, and nothing else. +- **Queue tier** (`queue`): the authoritative gate. The Linux build plus the + macOS build, the benchmark compile check, the Delta contrib build gate, the + PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg 1.8/1.9/1.10/1.11, + evaluated against the merge result rather than against the PR head. Every queue-only job has a `run-*` label that opts a pull request into it -early, listed in the diagram below. +early, listed in the diagram below. The Spark SQL and Iceberg suites used to +be split between the tiers, with Spark 4.1 (catalyst and `sql_core`) and +Iceberg 1.11 on every pull request. They moved behind the queue once +agent-driven review, and agent-driven replies to review, multiplied the +number of pushes a pull request goes through before it is queued: every one +of those pushes paid for the whole Spark 4.1 build and the Iceberg 1.11 run, +and the queue was going to run both anyway. `spark_3_4` is in neither tier. Spark 3.4 is deprecated, so its Spark SQL suite no longer gates a merge; it runs only when a pull request carries @@ -82,15 +87,17 @@ to `pr_build_linux.yml` without either the guard or an entry in PR + queue tier push to main only queue tier, or PR with label --------------- ----------------- --------------------------- pr_build_linux (+ push, cache only) docs pr_build_macos run-macos-tests - spark_4_1 (catalyst + sql_core) pr_benchmark_check run-benchmark-check - iceberg_1_11 delta_build_gate run-delta-build-gate + pr_benchmark_check run-benchmark-check + delta_build_gate run-delta-build-gate pyarrow_udf_test run-pyarrow-udf-tests - spark_4_1 sql_hive run-spark-4.1-hive-tests spark_3_5 run-spark-3.5-tests spark_4_0 run-spark-4.0-tests - label or dispatch only iceberg_1_8 run-iceberg-tests - ---------------------- iceberg_1_9 run-iceberg-tests - spark_3_4 run-spark-3.4-tests iceberg_1_10 run-iceberg-tests + spark_4_1 run-spark-4.1-tests + label or dispatch only spark_4_1 sql_hive run-spark-4.1-hive-tests + ---------------------- iceberg_1_8 run-iceberg-tests + spark_3_4 run-spark-3.4-tests iceberg_1_9 run-iceberg-tests + iceberg_1_10 run-iceberg-tests + iceberg_1_11 run-iceberg-tests | | | +-----------------------------------+-----------------------------------+ @@ -110,25 +117,25 @@ to `pr_build_linux.yml` without either the guard or an entry in ## What runs when -| Job in `ci.yml` | Triggered by | Routing rule | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | -| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | -| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full` | `dev/ci/compute-changes.py` | -| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | -| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | -| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | -| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | -| `spark_4_1` | PR or merge group, paths matched; the `sql_hive` shards only in the merge group **or** with `run-spark-4.1-hive-tests` | Spark 4.1 sources | -| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | -| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | -| `iceberg_1_11` | PR or merge group, paths matched | Iceberg sources | -| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `required_checks` | always, after every job above except `docs` | none (always runs) | +| Job in `ci.yml` | Triggered by | Routing rule | +| -------------------- | -------------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | +| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full` | `dev/ci/compute-changes.py` | +| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | +| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | +| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | +| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | +| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | +| `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | +| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | +| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | +| `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `required_checks` | always, after every job above except `docs` | none (always runs) | A heavy job appears in the PR's checks list as a `skipped` entry whenever its path filter or event criteria don't match. Skipped checks count as @@ -164,7 +171,12 @@ Two rules keep those runs from corrupting the PR's status: still going. Skipping the job would not help either, since a skipped check run still carries the name and still counts as passing. -`run-spark-4.1-tests` gates nothing: `spark_4_1` already runs on every PR. +`run-spark-4.1-tests` and `run-spark-4.1-hive-tests` both feed the single +`spark_4_1` call. The first sets both of its POLICY outputs and runs every +module; the second sets only `spark_4_1_hive` and runs only the `sql_hive` +shards. Adding the hive label to a pull request that already carries the suite +label re-runs just the hive rows, since a `labeled` run selects what the new +label gates. The opt-in labels have to exist in repository settings before they can be applied; `contains()` on a label nobody can add is simply always false, which diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4563b6eafc..a4dc427806 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,13 +22,12 @@ # Merging goes through GitHub's merge queue (see `rulesets` in `.asf.yaml`), so # there are two tiers: # -# pull_request fast feedback. Linux build + Spark 4.1 (catalyst and -# sql_core only) + Iceberg 1.11. -# merge_group the authoritative gate. Everything the PR tier runs, plus -# the macOS build, the benchmark compile check, the Delta -# contrib build gate, the PyArrow UDF suite, the Spark 4.1 -# sql_hive shards, Spark 3.5/4.0 and Iceberg 1.8/1.9/1.10, -# evaluated against the merge result rather than the PR head. +# pull_request fast feedback. The Linux build only. +# merge_group the authoritative gate. The Linux build plus the macOS +# build, the benchmark compile check, the Delta contrib build +# gate, the PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg +# 1.8/1.9/1.10/1.11, evaluated against the merge result +# rather than the PR head. # # Spark 3.4 is deprecated and sits outside both tiers: it runs only when a # pull request carries `run-spark-3.4-tests`, or from a manual dispatch. @@ -339,11 +338,12 @@ jobs: spark_4_1: name: Spark SQL Tests (Spark 4.1) needs: changes - # Two POLICY outputs feed one call, so the queue gets every module from a - # single 40-minute build instead of two. `spark_4_1` (PR tier) covers - # catalyst and the sql_core shards; `spark_4_1_hive` (queue-only, or the - # `run-spark-4.1-hive-tests` label) adds the sql_hive shards. A `labeled` - # run sets only the second, and then runs only the hive rows. + # Queue-only by default, like every other Spark SQL suite. Two POLICY + # outputs feed one call, so the queue gets every module from a single + # 40-minute build instead of two. `spark_4_1` covers catalyst and the + # sql_core shards; `spark_4_1_hive` adds the sql_hive shards. On a pull + # request `run-spark-4.1-tests` sets both, and `run-spark-4.1-hive-tests` + # sets only the second, which then runs only the hive rows. if: needs.changes.outputs.spark_4_1 == 'true' || needs.changes.outputs.spark_4_1_hive == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: @@ -384,8 +384,7 @@ jobs: iceberg_1_10: name: Iceberg Spark SQL Tests (Iceberg 1.10) needs: changes - # Queue-only by default; PRs need the `run-iceberg-tests` label. Iceberg 1.11 - # (Spark 4.1) is the PR-gated Iceberg job; 1.10 covers the Spark 3.5 path. + # Queue-only by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_10 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: @@ -398,7 +397,9 @@ jobs: iceberg_1_11: name: Iceberg Spark SQL Tests (Iceberg 1.11) needs: changes - # Runs on every PR: Iceberg 1.11 is our only Spark 4.1 Iceberg coverage. + # Queue-only by default; PRs need the `run-iceberg-tests` label. Iceberg + # 1.11 is our only Spark 4.1 Iceberg coverage, and it was the PR-tier + # Iceberg job for that reason until every suite moved behind the queue. if: needs.changes.outputs.iceberg_1_11 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 99e55e19f7..d56ac0d42e 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -136,14 +136,16 @@ # where "allowed" ignores path filters. Written out longhand rather than # derived from POLICY, so that a change to the routing has to be stated twice # and cannot be made by accident. -PR_TIER = {"build_linux", "build_linux_full", "spark_4_1", "iceberg_1_11"} -SPARK_OPT_IN = {"spark_3_5", "spark_4_0", "spark_4_1_hive"} +# The PR tier is the Linux build and nothing else. Every Spark SQL and Iceberg +# suite waits for the queue, or for its label. +PR_TIER = {"build_linux", "build_linux_full"} +SPARK_OPT_IN = {"spark_3_5", "spark_4_0", "spark_4_1", "spark_4_1_hive"} # Spark 3.4 is deprecated and sits outside the queue tier entirely: a label on # a pull request, or a workflow_dispatch, and nothing else. Keeping it in its # own set is what makes the `merge_group` case below assert its absence rather # than quietly accept it coming back. SPARK_DEPRECATED = {"spark_3_4"} -ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10"} +ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"} BUILD_OPT_IN = {"build_macos", "benchmark", "delta_gate", "pyarrow_udf"} QUEUE_TIER = PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN ALL_JOBS = QUEUE_TIER | SPARK_DEPRECATED | {"docs"} @@ -209,8 +211,22 @@ }, {"build_macos"}, ), - # The Spark 4.1 hive shards are queue-only with their own label. The label - # adds them to the PR tier's Spark 4.1 call rather than starting a second. + # Spark 4.1 is queue-only too. Two labels feed its one call: the suite + # label selects every module, the hive label only the sql_hive shards. + # Neither label pulls in any other Spark version. + ( + {"name": "pull_request", "action": "synchronize", "labels": ["run-spark-4.1-tests"]}, + PR_TIER | {"spark_4_1", "spark_4_1_hive"}, + ), + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-spark-4.1-tests", + "labels": ["run-spark-4.1-tests"], + }, + {"spark_4_1", "spark_4_1_hive"}, + ), ( {"name": "pull_request", "action": "synchronize", "labels": ["run-spark-4.1-hive-tests"]}, PR_TIER | {"spark_4_1_hive"}, @@ -224,6 +240,18 @@ }, {"spark_4_1_hive"}, ), + # Adding the hive label on top of the suite label re-runs only the hive + # rows: a `labeled` run selects what the new label gates, and the suite + # label's earlier run already covered every module at this commit. + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-spark-4.1-hive-tests", + "labels": ["run-spark-4.1-tests", "run-spark-4.1-hive-tests"], + }, + {"spark_4_1_hive"}, + ), # An opt-in label present on a pushed commit adds just that suite. For the # deprecated Spark 3.4 suite the label is the *only* way it ever runs on a # pull request or the queue, so this case and the `labeled` one below are diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 8a81f11702..1beb07f793 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -449,19 +449,28 @@ "spark_3_4": ["label:run-spark-3.4-tests"], "spark_3_5": ["queue", "label:run-spark-3.5-tests"], "spark_4_0": ["queue", "label:run-spark-4.0-tests"], - # Spark 4.1 is the default build profile, so it is the cheapest early - # warning that a change is wrong and stays in the PR tier. Only the - # catalyst and sql_core shards, though: over Aug 12 to Sep 11 2026 the - # three sql_hive shards cost about 65 runner-hours a day on pull requests - # and were the only failing job on 7 PR runs, against 33 for sql_core, and - # their 67-minute shard set the PR tier's wall clock. See issue #5870. - "spark_4_1": ["pr", "queue"], - "spark_4_1_hive": ["queue", "label:run-spark-4.1-hive-tests"], + # No Spark SQL suite runs on a plain pull request. Spark 4.1 was the last + # one in the PR tier, first whole (issue #5870 pulled the sql_hive shards + # out) and then catalyst and sql_core alone. What changed is how often a + # pull request is pushed: with agent-driven review and agent-driven + # replies to review, a PR now goes through several more rounds before it + # is queued, and each round paid for the whole 4.1 build. The queue still + # runs every shard before anything lands; the two labels bring the run + # forward. `run-spark-4.1-tests` selects the whole suite, so it appears on + # both outputs; `run-spark-4.1-hive-tests` selects only the hive shards. + "spark_4_1": ["queue", "label:run-spark-4.1-tests"], + "spark_4_1_hive": [ + "queue", + "label:run-spark-4.1-tests", + "label:run-spark-4.1-hive-tests", + ], + # Same for Iceberg: 1.11 was the PR-tier version because it is the only + # Spark 4.1 coverage, and it now waits for the queue with the other three. + # One label opts a pull request into all four. "iceberg_1_8": ["queue", "label:run-iceberg-tests"], "iceberg_1_9": ["queue", "label:run-iceberg-tests"], "iceberg_1_10": ["queue", "label:run-iceberg-tests"], - # Iceberg 1.11 is our only Spark 4.1 Iceberg coverage, so it is not opt-in. - "iceberg_1_11": ["pr", "queue"], + "iceberg_1_11": ["queue", "label:run-iceberg-tests"], } diff --git a/dev/ci/spark-sql-modules.py b/dev/ci/spark-sql-modules.py index f31ac25c48..ff644abd37 100644 --- a/dev/ci/spark-sql-modules.py +++ b/dev/ci/spark-sql-modules.py @@ -18,8 +18,8 @@ # The test matrix for .github/workflows/spark_sql_test_reusable.yml. # # The rows used to be a literal `strategy.matrix.module` list in the workflow. -# They live here so that a caller can ask for a subset: the umbrella keeps the -# Spark 4.1 `sql_hive` shards out of the PR tier (see POLICY in +# They live here so that a caller can ask for a subset: the umbrella lets a +# pull request opt into only the Spark 4.1 `sql_hive` shards (see POLICY in # dev/ci/compute-changes.py and issue #5870) and a job-level `if:` cannot see # `matrix`, so the selection has to happen before the matrix is expanded. The # `build` job runs this script and publishes the result as a job output that diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 4697ac60d2..aeff2bc013 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -35,9 +35,9 @@ Which jobs run also depends on the event: | Suite | Pull request | Merge queue | | ------------------------------------------------- | ------------ | ----------- | | Linux build, lint, and Comet test suites | yes | yes | -| Spark SQL tests, Spark 4.1, catalyst and sql_core | yes | yes | +| Spark SQL tests, Spark 4.1, catalyst and sql_core | with label | yes | | Spark SQL tests, Spark 4.1, sql_hive | with label | yes | -| Iceberg Spark SQL tests, Iceberg 1.11 | yes | yes | +| Iceberg Spark SQL tests, Iceberg 1.11 | with label | yes | | macOS build and Comet test suites | with label | yes | | Benchmark compile and lint check | with label | yes | | Delta contrib build gate | with label | yes | @@ -84,17 +84,18 @@ cases that pin both down. Each suite outside the PR tier has a label that runs it on a pull request: -| Label | Runs | -| -------------------------- | ---------------------------------------------------- | -| `run-macos-tests` | macOS build and Comet test suites | -| `run-benchmark-check` | Benchmark compile and lint check | -| `run-delta-build-gate` | Delta contrib build gate | -| `run-pyarrow-udf-tests` | PyArrow UDF tests against Spark 4.0/4.1/4.2 | -| `run-spark-4.1-hive-tests` | Spark SQL hive tests against Spark 4.1 | -| `run-spark-3.4-tests` | Spark SQL tests against Spark 3.4 | -| `run-spark-3.5-tests` | Spark SQL tests against Spark 3.5 | -| `run-spark-4.0-tests` | Spark SQL tests against Spark 4.0 | -| `run-iceberg-tests` | Iceberg Spark SQL tests against Iceberg 1.8/1.9/1.10 | +| Label | Runs | +| -------------------------- | ----------------------------------------------------- | +| `run-macos-tests` | macOS build and Comet test suites | +| `run-benchmark-check` | Benchmark compile and lint check | +| `run-delta-build-gate` | Delta contrib build gate | +| `run-pyarrow-udf-tests` | PyArrow UDF tests against Spark 4.0/4.1/4.2 | +| `run-spark-4.1-tests` | Spark SQL tests against Spark 4.1, every module | +| `run-spark-4.1-hive-tests` | Spark SQL tests against Spark 4.1, sql_hive only | +| `run-spark-3.4-tests` | Spark SQL tests against Spark 3.4 | +| `run-spark-3.5-tests` | Spark SQL tests against Spark 3.5 | +| `run-spark-4.0-tests` | Spark SQL tests against Spark 4.0 | +| `run-iceberg-tests` | Iceberg Spark SQL tests against every Iceberg version | For every suite except Spark 3.4 the label only brings the run forward; the queue would have run it anyway before the change landed. For Spark 3.4 the label is the only way the suite runs on a @@ -126,12 +127,15 @@ re-run the failed jobs from the Actions page, or remove and re-apply the label. Use a label when a change is likely to behave differently on a version or platform the PR tier does not cover. Some examples: +- a change to the serde, the planner, or a native operator, where the Comet test suites pass but + Spark's own SQL suite is the thing that would catch a behavior difference; `run-spark-4.1-tests` + runs it against the default profile - code under `spark/src/main/spark-3.4/`, `spark-3.5/`, `spark-4.0/` or the shared `spark-3.x/` directory, or any change to `CometExprShim` and friends -- a change to a Spark SQL diff under `dev/diffs/` for a version other than 4.1 +- a change to a Spark SQL diff under `dev/diffs/` - anything that touches Hive table support, `InsertIntoHiveTable`, or the `sql/hive` parts of the 4.1 diff -- anything touching Iceberg reflection or the Iceberg diffs +- anything touching Iceberg reflection, the Iceberg scan or write path, or the Iceberg diffs - native code with platform-specific behavior, or a dependency bump that changes what is compiled on macOS - a change to the benchmark sources under `spark/src/test/scala/org/apache/spark/sql/benchmark` diff --git a/docs/source/contributor-guide/iceberg-spark-tests.md b/docs/source/contributor-guide/iceberg-spark-tests.md index 65230eb497..8e45e43446 100644 --- a/docs/source/contributor-guide/iceberg-spark-tests.md +++ b/docs/source/contributor-guide/iceberg-spark-tests.md @@ -100,9 +100,9 @@ diff must be generated against its own tag. The `iceberg_spark_test_.yml` workflows apply these diffs and run the three Gradle targets above against each Iceberg version, all with Java 17. Iceberg 1.8.1 runs against Spark 3.4.3; Iceberg 1.9.1 and 1.10.0 -run against Spark 3.5.9; Iceberg 1.11.0 runs against Spark 4.1.3. Iceberg 1.11 -(the only version testing Spark 4.1) runs on every pull request and in the merge queue; the older versions -(1.8, 1.9, 1.10) run only in the merge queue, or on a pull request labeled `run-iceberg-tests`. All caller +run against Spark 3.5.9; Iceberg 1.11.0 runs against Spark 4.1.3. All four versions run in the +merge queue, or earlier on a pull request labeled `run-iceberg-tests`; none runs on an unlabeled +pull request. All caller workflows delegate to `iceberg_spark_test_reusable.yml`, which holds the build and test job logic. See [.github/workflows/README.md](https://github.com/apache/datafusion-comet/blob/main/.github/workflows/README.md) for how the pull-request and merge-queue tiers differ. @@ -129,6 +129,7 @@ combined candidate classes and executed test cases equal an unsharded run exactl checks nested, parameterized, inherited, and dynamically generated tests, existing exclusions, and failure propagation. The fixture does not compile Spark or Iceberg. -Apply the `run-iceberg-tests` label to a pull request whenever it touches reflection code -(`org.apache.comet.iceberg.IcebergReflection`) or other logic whose behavior can differ across Iceberg -versions, since Iceberg 1.11 alone will not catch a regression that only affects 1.8, 1.9, or 1.10. +Apply the `run-iceberg-tests` label to a pull request whenever it touches the Iceberg scan or write +path, reflection code (`org.apache.comet.iceberg.IcebergReflection`), or other logic whose behavior +can differ across Iceberg versions. The Comet test suites in the Linux build do not exercise Iceberg's +own Spark tests, so without the label the first Iceberg verdict is the merge queue's. diff --git a/docs/source/contributor-guide/spark-sql-tests.md b/docs/source/contributor-guide/spark-sql-tests.md index 3b83c227c7..c4834bcff9 100644 --- a/docs/source/contributor-guide/spark-sql-tests.md +++ b/docs/source/contributor-guide/spark-sql-tests.md @@ -169,8 +169,9 @@ git diff v3.5.6 > ../datafusion-comet/dev/diffs/3.5.6.diff ## Running Tests in CI The easiest way to run the tests is to open a pull request against Comet and let CI run them. Spark -4.1 runs on every pull request, and Spark 3.5 and 4.0 run in the merge queue or earlier with the -`run-spark-3.5-tests` / `run-spark-4.0-tests` labels. +3.5, 4.0 and 4.1 all run in the merge queue, or earlier on a pull request carrying the +`run-spark-3.5-tests` / `run-spark-4.0-tests` / `run-spark-4.1-tests` label. No Spark SQL suite +runs on an unlabeled pull request. Spark 3.4 is deprecated and is not run by the merge queue. It runs only when a pull request carries the `run-spark-3.4-tests` label, or when `ci.yml` is dispatched manually from the Actions page. From f17ddbeac61958983311f6c591fc39b95bae2119 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 00:58:18 -0600 Subject: [PATCH 2/8] ci: run the Linux test matrix against the default Spark profile only on pull requests The five Spark profiles in the linux-test matrix cost about the same each and together are three quarters of what the Linux build spends on a pull request. Run the Comet test suites against Spark 4.1 only on a pull request and against every profile in the merge queue, or on a pull request carrying the new `run-all-spark-profiles` label. The Lint Java matrix still compiles Spark 3.4/3.5/4.0 on every pull request, so a shim that fails to build is still caught there; only the runtime suites wait for the queue. A job-level `if:` cannot see `matrix`, so the profile rows move out of the workflow into dev/ci/linux-test-profiles.py with a tier each. The `lint` job runs it with a new `profiles` input and publishes the rows as a job output that the matrix reads with `fromJSON`, the same way the Spark SQL reusable workflow selects its module shards. A third POLICY output, `build_linux_all_profiles`, feeds that input from ci.yml; a `labeled` run sets only that output and runs just the profiles the PR tier skipped. check-ci-config.py asserts the two tiers partition the list, that the `pr` tier is exactly the default profile, and that ci.yml passes the input. --- .github/workflows/README.md | 96 +++++++++++-------- .github/workflows/ci.yml | 42 ++++++--- .github/workflows/pr_build_linux.yml | 60 +++++++----- dev/ci/check-ci-config.py | 89 +++++++++++++++++- dev/ci/compute-changes.py | 16 +++- dev/ci/linux-test-profiles.py | 97 ++++++++++++++++++++ docs/source/contributor-guide/ci.md | 9 +- docs/source/contributor-guide/development.md | 7 +- 8 files changed, 332 insertions(+), 84 deletions(-) create mode 100644 dev/ci/linux-test-profiles.py diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 19d479fe33..56a3de4bd6 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -15,20 +15,24 @@ Merging goes through GitHub's merge queue, configured by the `Merge Queue` ruleset in `.asf.yaml`. That splits CI into two tiers: - **PR tier** (`pr`): fast feedback while a change is being iterated on. - The Linux build, and nothing else. -- **Queue tier** (`queue`): the authoritative gate. The Linux build plus the - macOS build, the benchmark compile check, the Delta contrib build gate, the - PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg 1.8/1.9/1.10/1.11, + The Linux build, with the Comet test suites run against the default Spark + profile (4.1) only, and nothing else. +- **Queue tier** (`queue`): the authoritative gate. The Linux build with the + Comet test suites against every Spark profile, plus the macOS build, the + benchmark compile check, the Delta contrib build gate, the PyArrow UDF + suite, Spark 3.5/4.0/4.1 and Iceberg 1.8/1.9/1.10/1.11, evaluated against the merge result rather than against the PR head. Every queue-only job has a `run-*` label that opts a pull request into it -early, listed in the diagram below. The Spark SQL and Iceberg suites used to -be split between the tiers, with Spark 4.1 (catalyst and `sql_core`) and -Iceberg 1.11 on every pull request. They moved behind the queue once -agent-driven review, and agent-driven replies to review, multiplied the -number of pushes a pull request goes through before it is queued: every one -of those pushes paid for the whole Spark 4.1 build and the Iceberg 1.11 run, -and the queue was going to run both anyway. +early, listed in the diagram below. The PR tier used to be wider: Spark 4.1 +(catalyst and `sql_core`) and Iceberg 1.11 ran on every pull request, and the +Linux build ran its test matrix against all five Spark profiles. Those moved +behind the queue once agent-driven review, and agent-driven replies to +review, multiplied the number of pushes a pull request goes through before it +is queued: every one of those pushes paid for all of it, and the queue was +going to run all of it anyway. The Lint Java matrix still compiles Spark +3.4/3.5/4.0 on every pull request, so a shim that fails to build is still +caught there; only the runtime suites wait. `spark_3_4` is in neither tier. Spark 3.4 is deprecated, so its Spark SQL suite no longer gates a merge; it runs only when a pull request carries @@ -53,15 +57,25 @@ Maven and TPC-H/TPC-DS caches on `main` stale until the next unrelated change. Warming those caches is the only thing the push run is for, so on `push` the Linux build runs in **cache-refresh-only** mode: `build-native`, `linux-test-rust` and the two TPC-H/TPC-DS jobs, each stopping once its cache -entry is written, and nothing else. The lints, the 5x4 `linux-test` matrix and +entry is written, and nothing else. The lints, the `linux-test` matrix and the TPC query runs are skipped, which takes the push tier from 587 -runner-minutes to about 73. Two POLICY outputs express this: `build_linux` +runner-minutes to about 73. Three POLICY outputs express this: `build_linux` says whether the workflow runs at all, `build_linux_full` whether it runs the -lints and tests too, and `ci.yml` folds the second into the workflow's -`cache-refresh-only` input. `dev/ci/check-ci-config.py` fails if a job is added -to `pr_build_linux.yml` without either the guard or an entry in +lints and tests too, and `build_linux_all_profiles` whether the `linux-test` +matrix covers every Spark profile or only the default one. `ci.yml` folds the +second into the workflow's `cache-refresh-only` input and the third into its +`profiles` input. `dev/ci/check-ci-config.py` fails if a job is added to +`pr_build_linux.yml` without either the guard or an entry in `CACHE_REFRESH_JOBS` naming the cache it writes. See issue #5929. +The profile rows of the `linux-test` matrix live in +`dev/ci/linux-test-profiles.py` rather than in the workflow, because a +job-level `if:` cannot see `matrix`: the `lint` job runs the script with the +`profiles` input and publishes the rows as a job output that the matrix reads +with `fromJSON`. Each row carries a tier, `pr` for the default build profile +and `queue` for the other four, and `check-ci-config.py` asserts that the two +tiers partition the list and that the `pr` tier is exactly the default profile. + ``` pull_request | merge_group | push to main | workflow_dispatch | @@ -87,9 +101,11 @@ to `pr_build_linux.yml` without either the guard or an entry in PR + queue tier push to main only queue tier, or PR with label --------------- ----------------- --------------------------- pr_build_linux (+ push, cache only) docs pr_build_macos run-macos-tests - pr_benchmark_check run-benchmark-check + (Spark 4.1 profile only) pr_benchmark_check run-benchmark-check delta_build_gate run-delta-build-gate pyarrow_udf_test run-pyarrow-udf-tests + pr_build_linux run-all-spark-profiles + (other profiles) spark_3_5 run-spark-3.5-tests spark_4_0 run-spark-4.0-tests spark_4_1 run-spark-4.1-tests @@ -117,25 +133,25 @@ to `pr_build_linux.yml` without either the guard or an entry in ## What runs when -| Job in `ci.yml` | Triggered by | Routing rule | -| -------------------- | -------------------------------------------------------------------------------------------------------------- | ----------------------------------- | -| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | -| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full` | `dev/ci/compute-changes.py` | -| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | -| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | -| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | -| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | -| `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | -| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | -| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | -| `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `required_checks` | always, after every job above except `docs` | none (always runs) | +| Job in `ci.yml` | Triggered by | Routing rule | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------- | +| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | +| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full`; the test matrix's non-default Spark profiles only in the merge group **or** with `run-all-spark-profiles`, via `build_linux_all_profiles` | `dev/ci/compute-changes.py` | +| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | +| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | +| `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | +| `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | +| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | +| `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | +| `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | +| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | +| `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `required_checks` | always, after every job above except `docs` | none (always runs) | A heavy job appears in the PR's checks list as a `skipped` entry whenever its path filter or event criteria don't match. Skipped checks count as @@ -235,9 +251,11 @@ Moving a suite between the PR and queue tiers is a one-word edit to `POLICY`. An output does not have to map one-to-one onto a job. Two outputs can feed a single call when part of a workflow belongs in a different tier from the rest: `spark_4_1` / `spark_4_1_hive` select which module shards the one Spark 4.1 -build runs, and `build_linux` / `build_linux_full` select whether the Linux -build runs everything or only the jobs that populate `main`'s caches. Both -share their `FILTERS` list by assignment so the two entries cannot drift. +build runs, and `build_linux` / `build_linux_full` / `build_linux_all_profiles` +select whether the Linux build runs everything, only the jobs that populate +`main`'s caches, or the test matrix against every Spark profile rather than +the default one. Each group shares its `FILTERS` list by assignment so the +entries cannot drift. So adding a suite, moving sources, or changing when something runs is an edit to one of those two tables, not to ten `${{ }}` expressions. Keeping the policy diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4dc427806..9a72a3f21b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,10 +22,12 @@ # Merging goes through GitHub's merge queue (see `rulesets` in `.asf.yaml`), so # there are two tiers: # -# pull_request fast feedback. The Linux build only. -# merge_group the authoritative gate. The Linux build plus the macOS -# build, the benchmark compile check, the Delta contrib build -# gate, the PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg +# pull_request fast feedback. The Linux build only, with its test matrix +# run against the default Spark profile (4.1) alone. +# merge_group the authoritative gate. The Linux build with the test +# matrix against every Spark profile, plus the macOS build, +# the benchmark compile check, the Delta contrib build gate, +# the PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg # 1.8/1.9/1.10/1.11, evaluated against the merge result # rather than the PR head. # @@ -160,6 +162,7 @@ jobs: outputs: build_linux: ${{ steps.compute.outputs.build_linux }} build_linux_full: ${{ steps.compute.outputs.build_linux_full }} + build_linux_all_profiles: ${{ steps.compute.outputs.build_linux_all_profiles }} build_macos: ${{ steps.compute.outputs.build_macos }} benchmark: ${{ steps.compute.outputs.benchmark }} delta_gate: ${{ steps.compute.outputs.delta_gate }} @@ -236,16 +239,31 @@ jobs: pr_build_linux: name: PR Build (Linux) needs: changes - if: needs.changes.outputs.build_linux == 'true' - uses: ./.github/workflows/pr_build_linux.yml - # Two POLICY outputs feed one call, the same shape as spark_4_1 below. + # Three POLICY outputs feed one call, the same shape as spark_4_1 below. # `build_linux` decides whether the workflow runs at all; `build_linux_full` - # decides whether it runs the lints and the 5x4 test matrix as well as the - # jobs that populate main's actions/cache entries. Only push to main sets - # the first without the second, which is the whole point: the queue has - # already tested that tree, so the push run is there for the caches alone. + # whether it runs the lints and the test matrix as well as the jobs that + # populate main's actions/cache entries; `build_linux_all_profiles` whether + # the test matrix covers every Spark profile or only the default one. The + # combinations that occur: + # + # pull request linux, full -> profiles: pr + # ... with the label linux, full, all -> profiles: all + # `labeled` run all -> profiles: queue-only + # merge queue linux, full, all -> profiles: all + # push to main linux -> cache-refresh-only + # + # Only push to main sets `build_linux` without `build_linux_full`, which is + # the whole point: the queue has already tested that tree, so the push run + # is there for the caches alone. A `labeled` run sets only the third, and + # then runs just the profiles the PR tier skipped. + if: needs.changes.outputs.build_linux == 'true' || needs.changes.outputs.build_linux_all_profiles == 'true' + uses: ./.github/workflows/pr_build_linux.yml with: - cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' }} + cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' && needs.changes.outputs.build_linux_all_profiles != 'true' }} + profiles: >- + ${{ needs.changes.outputs.build_linux_all_profiles != 'true' && 'pr' + || needs.changes.outputs.build_linux_full != 'true' && 'queue-only' + || 'all' }} pr_build_macos: name: PR Build (macOS) diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index 8b15d829aa..0a4f47c0c3 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -22,7 +22,7 @@ name: PR Build (Linux) # # Two modes, selected by `cache-refresh-only`: # -# false the full pipeline: the lints, the 5x4 linux-test matrix, the Rust +# false the full pipeline: the lints, the linux-test matrix, the Rust # tests and the TPC-H/TPC-DS correctness runs. Pull requests and the # merge queue. # true only the jobs that write an actions/cache entry, and each only far @@ -32,6 +32,14 @@ name: PR Build (Linux) # next pull request. See the `build_linux` / `build_linux_full` # comments in dev/ci/compute-changes.py for why that tier exists. # +# Within the full pipeline, `profiles` picks which Spark profiles the +# linux-test matrix runs the Comet test suites against: `pr` is the default +# build profile alone, `all` is every profile, and `queue-only` is the rest. +# The rows live in dev/ci/linux-test-profiles.py, which the `lint` job runs so +# that the matrix can be built from its output. The queue runs `all`; a pull +# request runs `pr` unless it carries `run-all-spark-profiles`. See the +# `build_linux_all_profiles` comment in dev/ci/compute-changes.py. +# # Keeping both modes in this file is deliberate. The cache keys are the whole # point of the push tier, and a separate cache-warming workflow would have to # repeat every one of them; here they cannot drift apart. dev/ci/check-ci-config.py @@ -46,6 +54,15 @@ on: test. Set by ci.yml on push to main. type: boolean default: false + profiles: + # Which Spark profiles the linux-test matrix runs. `pr` is the default + # build profile alone, `all` is every profile, `queue-only` is every + # profile except the `pr` one (a `labeled` run, where the PR-tier row + # already ran at this commit). Rows and tiers are in + # dev/ci/linux-test-profiles.py. + description: 'Which Spark profiles linux-test runs: all, pr (the default build profile) or queue-only (the rest)' + type: string + default: 'all' env: RUST_VERSION: stable @@ -65,9 +82,18 @@ jobs: runs-on: ubuntu-24.04 container: image: amd64/rust + outputs: + # The `profile` dimension of the linux-test matrix, selected by the + # `profiles` input. Computed here because this job already gates + # linux-test and a job-level `if:` cannot see `matrix`. + profile-matrix: ${{ steps.profiles.outputs.matrix }} steps: - uses: actions/checkout@v7 + - name: Select linux-test profiles + id: profiles + run: python3 dev/ci/linux-test-profiles.py --profiles "${{ inputs.profiles }}" --github-output "$GITHUB_OUTPUT" + - name: Check Rust formatting run: | rustup component add rustfmt @@ -379,32 +405,18 @@ jobs: key: ${{ runner.os }}-cargo-debug-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }} linux-test: - needs: build-native + # `lint` is already upstream via build-native; it is listed here so this + # job can read its `profile-matrix` output. + needs: [lint, build-native] if: ${{ !inputs.cache-refresh-only }} strategy: matrix: - # the goal with these profiles is to get coverage of all Java, Scala, and Spark - # versions without testing all possible combinations, which would be overkill - profile: - - name: "Spark 3.4, JDK 17, Scala 2.12" - java_version: "17" - maven_opts: "-Pspark-3.4 -Pscala-2.12" - - - name: "Spark 3.5, JDK 17, Scala 2.13" - java_version: "17" - maven_opts: "-Pspark-3.5 -Pscala-2.13" - - - name: "Spark 4.0, JDK 21" - java_version: "21" - maven_opts: "-Pspark-4.0" - - - name: "Spark 4.1, JDK 17" - java_version: "17" - maven_opts: "-Pspark-4.1" - - - name: "Spark 4.2, JDK 17" - java_version: "17" - maven_opts: "-Pspark-4.2" + # The Spark profiles live in dev/ci/linux-test-profiles.py and are + # selected by the `profiles` input: the default build profile on a + # pull request, all five in the merge queue. The goal of the full list + # is coverage of every Java, Scala and Spark version without testing + # every combination. + profile: ${{ fromJSON(needs.lint.outputs.profile-matrix) }} # Suites are grouped by functional area into balanced buckets so that no test # job runs much longer than ~23 min. See # docs/superpowers/specs/2026-05-22-pr-build-consolidation-design.md for the diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index d56ac0d42e..3b140d27bd 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -88,6 +88,7 @@ BUILD_JOBS = { "build_linux", "build_linux_full", + "build_linux_all_profiles", "build_macos", "spark_3_4", "spark_3_5", @@ -116,7 +117,10 @@ ([".github/actions/upload-artifact-retry/action.yaml"], BUILD_JOBS), ([".github/actions/download-artifact-retry/action.yaml"], BUILD_JOBS), # The Maven bootstrap composite is called only from pr_build_linux.yml. - ([".github/actions/maven-bootstrap/action.yaml"], {"build_linux", "build_linux_full"}), + ( + [".github/actions/maven-bootstrap/action.yaml"], + {"build_linux", "build_linux_full", "build_linux_all_profiles"}, + ), # Spot checks that the additions above did not widen unrelated routes. (["docs/source/user-guide/overview.md"], {"docs"}), (["native/core/benches/parquet_read.rs"], {"benchmark"}), @@ -128,7 +132,13 @@ (["contrib/delta/native/src/lib.rs"], {"delta_gate"}), ( ["spark/src/test/resources/pyspark/test_pyarrow_udf.py"], - {"build_linux", "build_linux_full", "build_macos", "pyarrow_udf"}, + { + "build_linux", + "build_linux_full", + "build_linux_all_profiles", + "build_macos", + "pyarrow_udf", + }, ), ] @@ -146,7 +156,15 @@ # than quietly accept it coming back. SPARK_DEPRECATED = {"spark_3_4"} ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"} -BUILD_OPT_IN = {"build_macos", "benchmark", "delta_gate", "pyarrow_udf"} +# `build_linux_all_profiles` is the linux-test matrix's non-default Spark +# profiles: part of the Linux build's call, not a job of its own. +BUILD_OPT_IN = { + "build_macos", + "benchmark", + "build_linux_all_profiles", + "delta_gate", + "pyarrow_udf", +} QUEUE_TIER = PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN ALL_JOBS = QUEUE_TIER | SPARK_DEPRECATED | {"docs"} @@ -211,6 +229,24 @@ }, {"build_macos"}, ), + # The linux-test matrix's non-default Spark profiles are queue-only with + # their own label. On a pushed commit the label adds them to the PR tier's + # Linux build call (`profiles: all`); on the `labeled` event alone it is + # the only output set, and ci.yml turns that into `profiles: queue-only` + # so the default profile, which already ran at this commit, is not repeated. + ( + {"name": "pull_request", "action": "synchronize", "labels": ["run-all-spark-profiles"]}, + PR_TIER | {"build_linux_all_profiles"}, + ), + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-all-spark-profiles", + "labels": ["run-all-spark-profiles"], + }, + {"build_linux_all_profiles"}, + ), # Spark 4.1 is queue-only too. Two labels feed its one call: the suite # label selects every module, the hive label only the sql_hive shards. # Neither label pulls in any other Spark version. @@ -345,6 +381,9 @@ # whole job. CACHE_REFRESH_GUARD = re.compile(r"^ if:.*!\s*inputs\.cache-refresh-only") CACHE_REFRESH_INPUT = re.compile(r"^\s+cache-refresh-only:\s*\$\{\{") +# `profiles:` is passed as a folded scalar (`>-`) whose expression sits on the +# next line, so match the key alone. +PROFILES_INPUT = re.compile(r"^\s+profiles:\s*(>-|\$\{\{)") def load_filters(): @@ -382,6 +421,49 @@ def check_spark_sql_modules(): return not failures +def check_linux_test_profiles(): + """`--profiles pr` and `--profiles queue-only` must partition `--profiles all`. + + ci.yml maps `build_linux_full` and `build_linux_all_profiles` onto these + three values. A profile in neither tier would never run anywhere; one in + both would run twice in the queue. The `pr` tier also has to be the + default build profile and nothing else, which is the whole reason the + split exists. And the caller has to pass the input at all: its default is + `all`, so a dropped `with:` line quietly puts every profile back on the + pull request tier. + """ + spec = importlib.util.spec_from_file_location("linux_test_profiles", "dev/ci/linux-test-profiles.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + failures = [] + names = lambda rows: [row["name"] for row in rows] + everything = names(module.select("all")) + pr, queue_only = names(module.select("pr")), names(module.select("queue-only")) + if pr != ["Spark 4.1, JDK 17"]: + failures.append(f"the pr tier must be the default build profile alone, got {pr}") + if not queue_only: + failures.append("the queue-only tier is empty (see PROFILES in dev/ci/linux-test-profiles.py)") + if sorted(pr + queue_only) != sorted(everything): + failures.append( + f"pr {pr} + queue-only {queue_only} does not partition all {everything} " + f"(see PROFILES in dev/ci/linux-test-profiles.py)" + ) + if len(set(everything)) != len(everything): + failures.append(f"duplicate profile names in {everything}") + for row in module.select("all"): + if sorted(row) != ["java_version", "maven_opts", "name"]: + failures.append(f"profile {row['name']!r} must carry exactly name, java_version and maven_opts") + ci = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines() + if not any(PROFILES_INPUT.match(line) for line in ci): + failures.append( + "ci.yml never passes `profiles:` to pr_build_linux.yml. The input " + "defaults to all, so without it every pull request runs every profile again" + ) + for failure in failures: + print(f"linux test profiles: {failure}") + return not failures + + def check_change_filters(): module = load_filters() failures = [] @@ -768,6 +850,7 @@ def check_cache_refresh_scope(): ok = check_change_filters() ok = check_event_policy() and ok ok = check_spark_sql_modules() and ok + ok = check_linux_test_profiles() and ok ok = check_artifact_names() and ok ok = check_local_actions_have_checkout() and ok ok = check_required_checks() and ok diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 1beb07f793..5e21e2c68f 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -71,6 +71,10 @@ # `cache-refresh-only` input. Populated below, after the dict, so the two # lists cannot drift. "build_linux_full": [], + # A third POLICY decision on the same inputs: whether the linux-test matrix + # runs every Spark profile or only the PR-tier one. ci.yml folds it into + # the reusable workflow's `profiles` input. Populated below as well. + "build_linux_all_profiles": [], "build_macos": [ "native/**", "common/**", @@ -386,6 +390,7 @@ } FILTERS["spark_4_1_hive"] = FILTERS["spark_4_1"] FILTERS["build_linux_full"] = FILTERS["build_linux"] +FILTERS["build_linux_all_profiles"] = FILTERS["build_linux"] # Which events may run each job, independent of the path filters above. # @@ -415,7 +420,7 @@ # restore-keys prefix match. # # On push that is the *only* thing it is for. The queue already tested the - # exact tree that landed, so re-running the lints and the 5x4 linux-test + # exact tree that landed, so re-running the lints and the linux-test # matrix there tests nothing, and they are 514 of the 587 runner-minutes a # push run costs. The split below keeps the cache writers on push and moves # everything else behind `build_linux_full`. @@ -425,6 +430,15 @@ # input, so dropping "push" here is what trims the push tier down to the # jobs that write an actions/cache entry. See issue #5929. "build_linux_full": ["pr", "queue"], + # The linux-test matrix's Spark profiles other than the default one. The + # five profiles cost about the same each, roughly 2,300 runner-minutes a + # day apiece on pull requests in mid-September 2026, and together they + # were three quarters of the Linux build. A pull request runs the Comet + # test suites against Spark 4.1 only; the queue runs all five. The + # lint-java matrix still compiles Spark 3.4/3.5/4.0 on every pull request, + # so what waits for the queue is runtime behaviour, not a shim that fails + # to build. ci.yml turns this output into the workflow's `profiles` input. + "build_linux_all_profiles": ["queue", "label:run-all-spark-profiles"], # macOS runners are the scarcest capacity we have, and the Linux build # already covers rustfmt and the Rust/JVM compile on every PR. The label # is for a change that touches platform-specific code. diff --git a/dev/ci/linux-test-profiles.py b/dev/ci/linux-test-profiles.py new file mode 100644 index 0000000000..c2162aaf45 --- /dev/null +++ b/dev/ci/linux-test-profiles.py @@ -0,0 +1,97 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# The `profile` dimension of the `linux-test` matrix in +# .github/workflows/pr_build_linux.yml. +# +# The five Spark profiles cost about the same each, and together they are +# three quarters of what the Linux build spends on a pull request. Only one of +# them is the default build profile, so a pull request runs the Comet test +# suites against that one and the merge queue runs all five. A job-level `if:` +# cannot see `matrix`, so the selection has to happen before the matrix is +# expanded: the `lint` job runs this script and publishes the result as a job +# output that `linux-test` reads with `fromJSON`, the same way +# spark-sql-modules.py picks the Spark SQL shards. +# +# `lint-java` keeps its own literal profile list. It compiles every profile it +# can on every pull request (about five minutes each), which is what keeps a +# shim that fails to compile on Spark 3.x from reaching the queue; only the +# runtime suites move behind it. +# +# Usage: +# linux-test-profiles.py --profiles all|pr|queue-only --github-output $GITHUB_OUTPUT +# linux-test-profiles.py --profiles pr (prints the matrix JSON) + +import argparse +import json +import sys +from pathlib import Path + +# `tier` is what --profiles selects on: "pr" rows run on every pull request +# and in the queue, "queue" rows only in the queue (or with the +# `run-all-spark-profiles` label). The goal of the list is coverage of every +# Java, Scala and Spark version without testing every combination. +PROFILES = [ + {"name": "Spark 3.4, JDK 17, Scala 2.12", "java_version": "17", "maven_opts": "-Pspark-3.4 -Pscala-2.12", "tier": "queue"}, + {"name": "Spark 3.5, JDK 17, Scala 2.13", "java_version": "17", "maven_opts": "-Pspark-3.5 -Pscala-2.13", "tier": "queue"}, + {"name": "Spark 4.0, JDK 21", "java_version": "21", "maven_opts": "-Pspark-4.0", "tier": "queue"}, + # The default build profile, and the one a contributor builds locally. + {"name": "Spark 4.1, JDK 17", "java_version": "17", "maven_opts": "-Pspark-4.1", "tier": "pr"}, + {"name": "Spark 4.2, JDK 17", "java_version": "17", "maven_opts": "-Pspark-4.2", "tier": "queue"}, +] + +SELECTORS = ("all", "pr", "queue-only") + + +def select(profiles): + """Return the matrix rows for a --profiles value, or raise ValueError.""" + if profiles not in SELECTORS: + raise ValueError(f"--profiles must be one of {', '.join(SELECTORS)}, got {profiles!r}") + if profiles == "all": + rows = PROFILES + elif profiles == "pr": + rows = [row for row in PROFILES if row["tier"] == "pr"] + else: + rows = [row for row in PROFILES if row["tier"] == "queue"] + return [{key: value for key, value in row.items() if key != "tier"} for row in rows] + + +def main(argv): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--profiles", + default="all", + help="all, pr (the profiles every pull request runs) or queue-only (the ones it does not)", + ) + parser.add_argument("--github-output", type=Path, help="append matrix= to this $GITHUB_OUTPUT file") + args = parser.parse_args(argv) + try: + rows = select(args.profiles) + except ValueError as e: + print(f"error: {e}", file=sys.stderr) + return 2 + matrix = json.dumps(rows) + if args.github_output: + with args.github_output.open("a", encoding="utf-8") as out: + out.write(f"matrix={matrix}\n") + else: + print(matrix) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index aeff2bc013..28d1ccc992 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -34,7 +34,9 @@ Which jobs run also depends on the event: | Suite | Pull request | Merge queue | | ------------------------------------------------- | ------------ | ----------- | -| Linux build, lint, and Comet test suites | yes | yes | +| Linux build, lint, Rust tests, TPC-H/TPC-DS | yes | yes | +| Comet test suites, Spark 4.1 | yes | yes | +| Comet test suites, Spark 3.4 / 3.5 / 4.0 / 4.2 | with label | yes | | Spark SQL tests, Spark 4.1, catalyst and sql_core | with label | yes | | Spark SQL tests, Spark 4.1, sql_hive | with label | yes | | Iceberg Spark SQL tests, Iceberg 1.11 | with label | yes | @@ -87,6 +89,7 @@ Each suite outside the PR tier has a label that runs it on a pull request: | Label | Runs | | -------------------------- | ----------------------------------------------------- | | `run-macos-tests` | macOS build and Comet test suites | +| `run-all-spark-profiles` | Comet test suites against Spark 3.4 / 3.5 / 4.0 / 4.2 | | `run-benchmark-check` | Benchmark compile and lint check | | `run-delta-build-gate` | Delta contrib build gate | | `run-pyarrow-udf-tests` | PyArrow UDF tests against Spark 4.0/4.1/4.2 | @@ -131,7 +134,9 @@ does not cover. Some examples: Spark's own SQL suite is the thing that would catch a behavior difference; `run-spark-4.1-tests` runs it against the default profile - code under `spark/src/main/spark-3.4/`, `spark-3.5/`, `spark-4.0/` or the shared `spark-3.x/` - directory, or any change to `CometExprShim` and friends + directory, or any change to `CometExprShim` and friends; `run-all-spark-profiles` runs the Comet + test suites against every Spark version rather than 4.1 alone (the Lint Java matrix already + compiles the 3.4/3.5/4.0 profiles on every pull request, so this is for runtime differences) - a change to a Spark SQL diff under `dev/diffs/` - anything that touches Hive table support, `InsertIntoHiveTable`, or the `sql/hive` parts of the 4.1 diff diff --git a/docs/source/contributor-guide/development.md b/docs/source/contributor-guide/development.md index 86f7fca7d5..f825cde080 100644 --- a/docs/source/contributor-guide/development.md +++ b/docs/source/contributor-guide/development.md @@ -667,9 +667,10 @@ excluded and how to run it. Run a manual suite with: Only add a suite to that list with a good reason; the default is that a new suite runs in CI. -The macOS suites only run in the merge queue by default. See -[Continuous Integration](ci.md) for the two tiers and the labels that opt a pull request into a -queue-only suite. +On a pull request the Linux build runs these suites against the default Spark profile (4.1) only; +the merge queue runs them against every Spark profile, and the macOS suites only run in the merge +queue by default. See [Continuous Integration](ci.md) for the two tiers and the labels that opt a +pull request into a queue-only suite. ### Pre-PR Summary From dfe52eb81eadeb195a9f44b297bcac76f4193495 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 06:13:15 -0600 Subject: [PATCH 3/8] ci: drop the history of the PR tier from the workflows README Review feedback: the README should describe the current tiers, not how they got there. The Lint Java sentence stays because it is current state. --- .github/workflows/README.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 56a3de4bd6..93e2b0374d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -24,15 +24,9 @@ ruleset in `.asf.yaml`. That splits CI into two tiers: evaluated against the merge result rather than against the PR head. Every queue-only job has a `run-*` label that opts a pull request into it -early, listed in the diagram below. The PR tier used to be wider: Spark 4.1 -(catalyst and `sql_core`) and Iceberg 1.11 ran on every pull request, and the -Linux build ran its test matrix against all five Spark profiles. Those moved -behind the queue once agent-driven review, and agent-driven replies to -review, multiplied the number of pushes a pull request goes through before it -is queued: every one of those pushes paid for all of it, and the queue was -going to run all of it anyway. The Lint Java matrix still compiles Spark -3.4/3.5/4.0 on every pull request, so a shim that fails to build is still -caught there; only the runtime suites wait. +early, listed in the diagram below. The Lint Java matrix compiles Spark +3.4/3.5/4.0 on every pull request, so a shim that fails to build is caught +there; only the runtime suites wait for the queue. `spark_3_4` is in neither tier. Spark 3.4 is deprecated, so its Spark SQL suite no longer gates a merge; it runs only when a pull request carries From a9f78484cf1732a1b5ff5406d70fafb08d5eb36b Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 09:43:54 -0600 Subject: [PATCH 4/8] ci: run the non-default Spark and Iceberg suites nightly instead of in the merge queue The merge queue ran three Spark SQL versions, four Iceberg versions and the Linux test matrix against every Spark profile on each merge. Spark 3.5, Spark 4.0 and Iceberg 1.8/1.9/1.10 were about 870 of the 1,900 runner-minutes a queue run cost, and the old Iceberg versions were the most common reason a queue run went red on a tree that was fine. Add a "nightly" tier to POLICY, fired by a schedule trigger on ci.yml at 06:00 UTC. The queue keeps one Spark version (4.1, the default profile) and one Iceberg version (1.11, the only Spark 4.1 coverage). Everything else moves to the nightly run: Spark SQL on 3.5 and 4.0, Iceberg 1.8/1.9/1.10 and the linux-test matrix's non-default Spark profiles. Their run-* labels still opt a pull request in early. The nightly run tests main as it stands, so it skips itself when nothing has landed since the previous one. A red nightly has no pull request to appear on, so a nightly_report job opens an issue labelled ci-nightly-failure listing the failed jobs, or comments on the one already open. The linux-test profile selector queue-only is renamed nightly, since those rows no longer run in the queue. check-ci-config.py gains a NIGHTLY_TIER set and a schedule case so a suite drifting back into the queue fails preflight. --- .github/workflows/README.md | 118 ++++++++++----- .github/workflows/ci.yml | 140 +++++++++++++++--- .github/workflows/pr_build_linux.yml | 22 +-- dev/ci/check-ci-config.py | 66 ++++++--- dev/ci/compute-changes.py | 71 ++++++--- dev/ci/linux-test-profiles.py | 30 ++-- docs/source/contributor-guide/ci.md | 85 +++++++---- docs/source/contributor-guide/development.md | 8 +- .../contributor-guide/iceberg-spark-tests.md | 11 +- .../contributor-guide/spark-sql-tests.md | 8 +- 10 files changed, 391 insertions(+), 168 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 93e2b0374d..c7fc5f174a 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -12,23 +12,28 @@ then fans out to the long-running test/build workflows. Each long workflow is a `workflow_call` reusable invoked from the umbrella. Merging goes through GitHub's merge queue, configured by the `Merge Queue` -ruleset in `.asf.yaml`. That splits CI into two tiers: +ruleset in `.asf.yaml`. That splits CI into three tiers: - **PR tier** (`pr`): fast feedback while a change is being iterated on. The Linux build, with the Comet test suites run against the default Spark profile (4.1) only, and nothing else. -- **Queue tier** (`queue`): the authoritative gate. The Linux build with the - Comet test suites against every Spark profile, plus the macOS build, the - benchmark compile check, the Delta contrib build gate, the PyArrow UDF - suite, Spark 3.5/4.0/4.1 and Iceberg 1.8/1.9/1.10/1.11, - evaluated against the merge result rather than against the PR head. - -Every queue-only job has a `run-*` label that opts a pull request into it -early, listed in the diagram below. The Lint Java matrix compiles Spark -3.4/3.5/4.0 on every pull request, so a shim that fails to build is caught -there; only the runtime suites wait for the queue. - -`spark_3_4` is in neither tier. Spark 3.4 is deprecated, so its Spark SQL +- **Queue tier** (`queue`): the authoritative gate. The PR tier plus the + macOS build, the benchmark compile check, the Delta contrib build gate, the + PyArrow UDF suite, Spark SQL on Spark 4.1 and Iceberg 1.11, evaluated + against the merge result rather than against the PR head. One Spark version + and one Iceberg version, both the default profile's. +- **Nightly tier** (`nightly`): the regression sweep of everything else, once + a day against `main` as it stands. The Comet test suites against the other + four Spark profiles, Spark SQL on Spark 3.5 and 4.0, and Iceberg + 1.8/1.9/1.10. See [Nightly tier](#nightly-tier) below for how a failure + surfaces. + +Every queue-only and nightly job has a `run-*` label that opts a pull request +into it early, listed in the diagram below. The Lint Java matrix compiles +Spark 3.4/3.5/4.0 on every pull request, so a shim that fails to build is +caught there; only the runtime suites wait for the queue or the nightly. + +`spark_3_4` is in none of the tiers. Spark 3.4 is deprecated, so its Spark SQL suite no longer gates a merge; it runs only when a pull request carries `run-spark-3.4-tests`, or from a `workflow_dispatch`. Anyone who wants to check a change against 3.4 can still do so, but note when that result starts @@ -67,11 +72,11 @@ The profile rows of the `linux-test` matrix live in job-level `if:` cannot see `matrix`: the `lint` job runs the script with the `profiles` input and publishes the rows as a job output that the matrix reads with `fromJSON`. Each row carries a tier, `pr` for the default build profile -and `queue` for the other four, and `check-ci-config.py` asserts that the two +and `nightly` for the other four, and `check-ci-config.py` asserts that the two tiers partition the list and that the `pr` tier is exactly the default profile. ``` - pull_request | merge_group | push to main | workflow_dispatch + pull_request | merge_group | push to main | schedule | workflow_dispatch | v +-----------------------+ @@ -98,16 +103,19 @@ tiers partition the list and that the `pr` tier is exactly the default profile. (Spark 4.1 profile only) pr_benchmark_check run-benchmark-check delta_build_gate run-delta-build-gate pyarrow_udf_test run-pyarrow-udf-tests - pr_build_linux run-all-spark-profiles - (other profiles) - spark_3_5 run-spark-3.5-tests - spark_4_0 run-spark-4.0-tests spark_4_1 run-spark-4.1-tests label or dispatch only spark_4_1 sql_hive run-spark-4.1-hive-tests - ---------------------- iceberg_1_8 run-iceberg-tests - spark_3_4 run-spark-3.4-tests iceberg_1_9 run-iceberg-tests - iceberg_1_10 run-iceberg-tests - iceberg_1_11 run-iceberg-tests + ---------------------- iceberg_1_11 run-iceberg-tests + spark_3_4 run-spark-3.4-tests + nightly tier, or PR with label + ------------------------------ + pr_build_linux run-all-spark-profiles + (other profiles) + spark_3_5 run-spark-3.5-tests + spark_4_0 run-spark-4.0-tests + iceberg_1_8 run-iceberg-tests + iceberg_1_9 run-iceberg-tests + iceberg_1_10 run-iceberg-tests | | | +-----------------------------------+-----------------------------------+ @@ -116,6 +124,14 @@ tiers partition the list and that the `pr` tier is exactly the default profile. | required_checks | ubuntu-slim | one flat name that | | is safe to require | + +-----------+-----------+ + | schedule only, when red + v + +-----------------------+ + | nightly_report | ubuntu-slim + | opens or comments on | + | a ci-nightly-failure | + | issue | +-----------------------+ reusable workflows invoked via `uses:`: @@ -129,23 +145,24 @@ tiers partition the list and that the `pr` tier is exactly the default profile. | Job in `ci.yml` | Triggered by | Routing rule | | -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------- | -| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) | -| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` | -| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full`; the test matrix's non-default Spark profiles only in the merge group **or** with `run-all-spark-profiles`, via `build_linux_all_profiles` | `dev/ci/compute-changes.py` | +| `preflight` | every PR / merge group / push / schedule / dispatch / label | none (always runs) | +| `changes` | every PR / merge group / push / schedule / dispatch / label | runs `dev/ci/compute-changes.py` | +| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full`; the test matrix's non-default Spark profiles only in the nightly run **or** with `run-all-spark-profiles`, via `build_linux_all_profiles` | `dev/ci/compute-changes.py` | | `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` | | `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | | `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | | `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | | `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | -| `spark_3_5` | merge group, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | +| `spark_3_5` | nightly, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | | `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | | `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | -| `spark_4_0` | merge group, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | +| `spark_4_0` | nightly, **or** PR with `run-spark-4.0-tests` | Spark 4.0 sources | | `iceberg_1_11` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_8` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_9` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | -| `iceberg_1_10` | merge group, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_8` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_9` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | +| `iceberg_1_10` | nightly, **or** PR with `run-iceberg-tests` | Iceberg sources | | `required_checks` | always, after every job above except `docs` | none (always runs) | +| `nightly_report` | schedule only, after `required_checks`, when it is not green | none | A heavy job appears in the PR's checks list as a `skipped` entry whenever its path filter or event criteria don't match. Skipped checks count as @@ -192,6 +209,31 @@ The opt-in labels have to exist in repository settings before they can be applied; `contains()` on a label nobody can add is simply always false, which makes the escape hatch look like it silently does nothing. +### Nightly tier + +`ci.yml` also fires on a `schedule` (06:00 UTC daily). On that event `changes` +has nothing to diff, so every path filter counts as matched and `POLICY` +selects the `nightly` tier alone: the queue already ran every `queue` job +against the tree that is now `main`, so repeating them would be paying twice. +The run skips itself when `HEAD` is older than the schedule interval (plus 30 +minutes of slack for scheduling jitter), so a quiet weekend does not re-test +the same tree three times. If a nightly fails for an infrastructure reason and +nothing lands the next day, dispatch `ci.yml` by hand. + +A red nightly has no pull request to appear on, and GitHub only emails a +scheduled run's failure to whoever last touched the workflow file, so +`nightly_report` runs after `required_checks` on the scheduled event and, when +the aggregate is not green, opens an issue labelled `ci-nightly-failure` listing +the jobs that failed. If one is already open it comments there instead, so +consecutive red nights accumulate in one issue. Closing the issue is how the +failure is acknowledged; the next red night opens a new one. The label has to +exist in repository settings, like the `run-*` labels above. + +The nightly run is keyed on the same sha as the push-to-main run of the commit +at the tip of `main`, so the concurrency group gives it its own `nightly` +subgroup; otherwise a merge landing just before the schedule fired would have +its cache refresh cancelled. + ## Standalone workflows (not under the umbrella) These workflows have their own triggers because they fire on events the @@ -236,11 +278,15 @@ in `dev/ci/compute-changes.py`: dorny/picomatch (`**` spans path segments, `*` stays within one, a leading `!` excludes). - **`POLICY`** — which events may run it. `"pr"` for every pull request, - `"queue"` for the merge queue, `"push"` for push to main, `"label:"` - for opt-in on a labelled pull request. `"pr"` and `"label:"` are mutually - exclusive. `workflow_dispatch` always runs everything. - -Moving a suite between the PR and queue tiers is a one-word edit to `POLICY`. + `"queue"` for the merge queue, `"nightly"` for the scheduled run, `"push"` + for push to main, `"label:"` for opt-in on a labelled pull request. + `"pr"` and `"label:"` are mutually exclusive. `workflow_dispatch` always + runs everything. + +Moving a suite between the PR, queue and nightly tiers is a one-word edit to +`POLICY`, plus the matching entry in `PR_TIER`, `QUEUE_TIER` or `NIGHTLY_TIER` +in `dev/ci/check-ci-config.py`, which is written out longhand on purpose so +that a tier change has to be stated twice. An output does not have to map one-to-one onto a job. Two outputs can feed a single call when part of a workflow belongs in a different tier from the rest: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a72a3f21b..fe3e942ae3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,19 +20,25 @@ # change touched files relevant to that workflow. # # Merging goes through GitHub's merge queue (see `rulesets` in `.asf.yaml`), so -# there are two tiers: +# there are three tiers: # # pull_request fast feedback. The Linux build only, with its test matrix # run against the default Spark profile (4.1) alone. -# merge_group the authoritative gate. The Linux build with the test -# matrix against every Spark profile, plus the macOS build, +# merge_group the authoritative gate. The PR tier plus the macOS build, # the benchmark compile check, the Delta contrib build gate, -# the PyArrow UDF suite, Spark 3.5/4.0/4.1 and Iceberg -# 1.8/1.9/1.10/1.11, evaluated against the merge result -# rather than the PR head. +# the PyArrow UDF suite, Spark SQL on Spark 4.1 and Iceberg +# 1.11, evaluated against the merge result rather than the +# PR head. One Spark version and one Iceberg version, both +# the default profile's. +# schedule the nightly regression sweep of everything else: the Linux +# test matrix against the other Spark profiles, Spark SQL on +# Spark 3.5 and 4.0, and Iceberg 1.8/1.9/1.10, against main +# as it stands. It skips itself when nothing has landed since +# the previous night, and a failure opens (or comments on) an +# issue labelled `ci-nightly-failure`. # -# Spark 3.4 is deprecated and sits outside both tiers: it runs only when a -# pull request carries `run-spark-3.4-tests`, or from a manual dispatch. +# Spark 3.4 is deprecated and sits outside all three tiers: it runs only when +# a pull request carries `run-spark-3.4-tests`, or from a manual dispatch. # # Which tier a job sits in is POLICY in dev/ci/compute-changes.py, not an # expression here. Heavy jobs deliberately have no `push` tier: the queue @@ -51,8 +57,11 @@ name: CI # opened/synchronize run. Keying the group on the label name keeps labeled runs # in their own subgroup so they never cancel the real commit run; opened and # synchronize both map to `commit` so a new push still supersedes its predecessor. +# The nightly run is keyed on the same sha as the push-to-main run of the commit +# at the tip of main, so it gets its own subgroup too; otherwise a merge landing +# just before the schedule fires would have its cache refresh cancelled. concurrency: - group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event.action == 'labeled' && github.event.label.name || 'commit' }} + group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event_name == 'schedule' && 'nightly' || github.event.action == 'labeled' && github.event.label.name || 'commit' }} cancel-in-progress: true on: @@ -62,6 +71,11 @@ on: push: branches: - main + schedule: + # 06:00 UTC daily: after the evening's merges in the Americas and before + # the working day starts in Europe, so a red nightly is waiting when the + # first people look. Miri runs at 04:00 and the snapshot publish at 03:00. + - cron: '0 6 * * *' workflow_dispatch: jobs: @@ -206,6 +220,20 @@ jobs: # every output true for this event so a manual run can exercise # any gated job. : + elif [[ "$EVENT_NAME" == "schedule" ]]; then + # Same: the nightly tests main as it stands, so there is nothing + # to diff and compute-changes.py runs the whole nightly tier. It + # skips when HEAD predates the previous night's run, so a quiet + # weekend does not re-test the same tree three times. The slack + # covers scheduling jitter; if a nightly fails for an + # infrastructure reason and nothing lands the next day, trigger + # ci.yml by hand instead. + head_ts=$(git log -1 --format=%ct) + age=$(( $(date +%s) - head_ts )) + if [[ "$age" -gt $(( 24 * 3600 + 1800 )) ]]; then + echo "HEAD is $(( age / 3600 ))h old; nothing has landed since the last nightly" + export NIGHTLY_STALE=true + fi elif [[ "$EVENT_NAME" == "pull_request" ]]; then git diff --name-only "$PR_BASE_SHA"..."$PR_HEAD_SHA" > changed_files.txt elif [[ "$EVENT_NAME" == "merge_group" ]]; then @@ -248,21 +276,22 @@ jobs: # # pull request linux, full -> profiles: pr # ... with the label linux, full, all -> profiles: all - # `labeled` run all -> profiles: queue-only - # merge queue linux, full, all -> profiles: all + # `labeled` run all -> profiles: nightly + # merge queue linux, full -> profiles: pr + # nightly all -> profiles: nightly # push to main linux -> cache-refresh-only # # Only push to main sets `build_linux` without `build_linux_full`, which is # the whole point: the queue has already tested that tree, so the push run - # is there for the caches alone. A `labeled` run sets only the third, and - # then runs just the profiles the PR tier skipped. + # is there for the caches alone. A `labeled` run and the nightly set only + # the third, and then run just the profiles the PR and queue tiers skip. if: needs.changes.outputs.build_linux == 'true' || needs.changes.outputs.build_linux_all_profiles == 'true' uses: ./.github/workflows/pr_build_linux.yml with: cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' && needs.changes.outputs.build_linux_all_profiles != 'true' }} profiles: >- ${{ needs.changes.outputs.build_linux_all_profiles != 'true' && 'pr' - || needs.changes.outputs.build_linux_full != 'true' && 'queue-only' + || needs.changes.outputs.build_linux_full != 'true' && 'nightly' || 'all' }} pr_build_macos: @@ -332,7 +361,7 @@ jobs: spark_3_5: name: Spark SQL Tests (Spark 3.5) needs: changes - # Queue-only by default; PRs need the `run-spark-3.5-tests` label. + # Nightly by default; PRs need the `run-spark-3.5-tests` label. if: needs.changes.outputs.spark_3_5 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: @@ -343,9 +372,8 @@ jobs: spark_4_0: name: Spark SQL Tests (Spark 4.0) needs: changes - # Queue-only by default; PRs need the `run-spark-4.0-tests` label. Swapped - # with spark_4_1 on the `oom` branch to validate the memory caps against - # Spark 4.1 by default. + # Nightly by default; PRs need the `run-spark-4.0-tests` label. Spark 4.1 + # is the one Spark SQL suite the queue runs, being the default profile. if: needs.changes.outputs.spark_4_0 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: @@ -376,7 +404,7 @@ jobs: iceberg_1_8: name: Iceberg Spark SQL Tests (Iceberg 1.8) needs: changes - # Queue-only by default; PRs need the `run-iceberg-tests` label. + # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_8 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: @@ -389,7 +417,7 @@ jobs: iceberg_1_9: name: Iceberg Spark SQL Tests (Iceberg 1.9) needs: changes - # Queue-only by default; PRs need the `run-iceberg-tests` label. + # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_9 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: @@ -402,7 +430,7 @@ jobs: iceberg_1_10: name: Iceberg Spark SQL Tests (Iceberg 1.10) needs: changes - # Queue-only by default; PRs need the `run-iceberg-tests` label. + # Nightly by default; PRs need the `run-iceberg-tests` label. if: needs.changes.outputs.iceberg_1_10 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: @@ -416,8 +444,8 @@ jobs: name: Iceberg Spark SQL Tests (Iceberg 1.11) needs: changes # Queue-only by default; PRs need the `run-iceberg-tests` label. Iceberg - # 1.11 is our only Spark 4.1 Iceberg coverage, and it was the PR-tier - # Iceberg job for that reason until every suite moved behind the queue. + # 1.11 is our only Spark 4.1 Iceberg coverage, which is why it is the one + # Iceberg version the queue runs while 1.8/1.9/1.10 run nightly. if: needs.changes.outputs.iceberg_1_11 == 'true' uses: ./.github/workflows/iceberg_spark_test_reusable.yml with: @@ -493,3 +521,69 @@ jobs: run: | echo "::error::One or more upstream jobs did not succeed. See the results above." exit 1 + + # --------------------------------------------------------------------------- + # nightly_report: a red nightly has no pull request to show up on, and a + # scheduled run only emails whoever last touched the workflow file, so + # without this a regression in the nightly tier sits unnoticed on the + # Actions tab. On the scheduled event, and only when `required_checks` is + # not green, it opens an issue labelled `ci-nightly-failure` listing the + # failed jobs, or comments on the one that is already open so consecutive + # red nights accumulate in one place instead of one issue a day. Closing + # the issue is how the failure is acknowledged; the next red night opens a + # new one. + # + # It sits downstream of the aggregator rather than of the nightly jobs + # themselves so the job list here does not have to be kept in step with + # POLICY. It is exempt from `required_checks.needs` in + # dev/ci/check-ci-config.py for the same reason `docs` is: it is never part + # of a merge decision. + # --------------------------------------------------------------------------- + nightly_report: + name: Nightly failure report + needs: required_checks + if: always() && github.event_name == 'schedule' && needs.required_checks.result != 'success' + permissions: + actions: read + issues: write + runs-on: ubuntu-slim + steps: + - uses: actions/github-script@v9 + with: + script: | + const label = 'ci-nightly-failure'; + const { owner, repo } = context.repo; + const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; + const shortSha = context.sha.slice(0, 10); + const day = new Date().toISOString().slice(0, 10); + + const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRun, { + owner, repo, run_id: context.runId, filter: 'latest', per_page: 100, + }); + const failed = jobs + .filter((job) => job.conclusion === 'failure' || job.conclusion === 'cancelled') + .map((job) => `- [${job.name}](${job.html_url}) (${job.conclusion})`); + + const body = [ + `The nightly CI run against \`main\` at ${shortSha} did not pass: ${runUrl}`, + '', + failed.length ? 'Jobs that did not succeed:' : 'No individual job reported a failure; see the run for details.', + ...failed, + '', + 'The nightly tier runs the Spark and Iceberg versions the merge queue does not', + '(see `POLICY` in `dev/ci/compute-changes.py`). Close this issue once the failure', + 'is understood; the next red nightly opens a new one.', + ].join('\n'); + + const { data: open } = await github.rest.issues.listForRepo({ + owner, repo, labels: label, state: 'open', per_page: 1, + }); + if (open.length > 0) { + await github.rest.issues.createComment({ owner, repo, issue_number: open[0].number, body }); + core.info(`Commented on #${open[0].number}`); + } else { + const { data: issue } = await github.rest.issues.create({ + owner, repo, title: `Nightly CI failed on ${day}`, body, labels: [label], + }); + core.info(`Opened #${issue.number}`); + } diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index 0a4f47c0c3..77fdb2de00 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -34,11 +34,12 @@ name: PR Build (Linux) # # Within the full pipeline, `profiles` picks which Spark profiles the # linux-test matrix runs the Comet test suites against: `pr` is the default -# build profile alone, `all` is every profile, and `queue-only` is the rest. +# build profile alone, `all` is every profile, and `nightly` is the rest. # The rows live in dev/ci/linux-test-profiles.py, which the `lint` job runs so -# that the matrix can be built from its output. The queue runs `all`; a pull -# request runs `pr` unless it carries `run-all-spark-profiles`. See the -# `build_linux_all_profiles` comment in dev/ci/compute-changes.py. +# that the matrix can be built from its output. A pull request and the merge +# queue run `pr`, the nightly run `nightly`, and a pull request carrying +# `run-all-spark-profiles` runs `all`. See the `build_linux_all_profiles` +# comment in dev/ci/compute-changes.py. # # Keeping both modes in this file is deliberate. The cache keys are the whole # point of the push tier, and a separate cache-warming workflow would have to @@ -56,11 +57,11 @@ on: default: false profiles: # Which Spark profiles the linux-test matrix runs. `pr` is the default - # build profile alone, `all` is every profile, `queue-only` is every - # profile except the `pr` one (a `labeled` run, where the PR-tier row - # already ran at this commit). Rows and tiers are in - # dev/ci/linux-test-profiles.py. - description: 'Which Spark profiles linux-test runs: all, pr (the default build profile) or queue-only (the rest)' + # build profile alone, `all` is every profile, `nightly` is every + # profile except the `pr` one (the nightly run, and a `labeled` run, + # where the PR-tier row already ran at this commit). Rows and tiers + # are in dev/ci/linux-test-profiles.py. + description: 'Which Spark profiles linux-test runs: all, pr (the default build profile) or nightly (the rest)' type: string default: 'all' @@ -413,7 +414,8 @@ jobs: matrix: # The Spark profiles live in dev/ci/linux-test-profiles.py and are # selected by the `profiles` input: the default build profile on a - # pull request, all five in the merge queue. The goal of the full list + # pull request and in the merge queue, the other four in the nightly + # run. The goal of the full list # is coverage of every Java, Scala and Spark version without testing # every combination. profile: ${{ fromJSON(needs.lint.outputs.profile-matrix) }} diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 3b140d27bd..ca43442c59 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -79,8 +79,9 @@ # Jobs that legitimately stay out of the aggregator's `needs:`. `docs` deploys # to asf-site on push to main; it gates nothing and is never part of a merge # decision, so folding it in would only turn a failed site deploy into a red -# `Required Checks` on main. -AGGREGATOR_EXEMPT = {AGGREGATOR_JOB, "docs"} +# `Required Checks` on main. `nightly_report` runs *after* the aggregator, on +# the scheduled event only, to open an issue when the nightly tier fails. +AGGREGATOR_EXEMPT = {AGGREGATOR_JOB, "docs", "nightly_report"} # Changed-file list -> the set of outputs compute-changes.py must report true. # Every other output must be false. Keep one case per shared build input so a @@ -147,13 +148,14 @@ # derived from POLICY, so that a change to the routing has to be stated twice # and cannot be made by accident. # The PR tier is the Linux build and nothing else. Every Spark SQL and Iceberg -# suite waits for the queue, or for its label. +# suite waits for the queue or the nightly run, or for its label. PR_TIER = {"build_linux", "build_linux_full"} SPARK_OPT_IN = {"spark_3_5", "spark_4_0", "spark_4_1", "spark_4_1_hive"} -# Spark 3.4 is deprecated and sits outside the queue tier entirely: a label on -# a pull request, or a workflow_dispatch, and nothing else. Keeping it in its -# own set is what makes the `merge_group` case below assert its absence rather -# than quietly accept it coming back. +# Spark 3.4 is deprecated and sits outside the queue and nightly tiers +# entirely: a label on a pull request, or a workflow_dispatch, and nothing +# else. Keeping it in its own set is what makes the `merge_group` and +# `schedule` cases below assert its absence rather than quietly accept it +# coming back. SPARK_DEPRECATED = {"spark_3_4"} ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"} # `build_linux_all_profiles` is the linux-test matrix's non-default Spark @@ -165,16 +167,34 @@ "delta_gate", "pyarrow_udf", } -QUEUE_TIER = PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN -ALL_JOBS = QUEUE_TIER | SPARK_DEPRECATED | {"docs"} +# The queue runs one Spark version (4.1, the default profile) and one Iceberg +# version (1.11, the only Spark 4.1 coverage). Every other Spark and Iceberg +# version, and the linux-test matrix's other Spark profiles, run once a night +# against main instead. Spelled out as the set of jobs the queue must *not* +# run, so a suite drifting back into the queue fails the `merge_group` case. +NIGHTLY_TIER = { + "spark_3_5", + "spark_4_0", + "iceberg_1_8", + "iceberg_1_9", + "iceberg_1_10", + "build_linux_all_profiles", +} +QUEUE_TIER = (PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN) - NIGHTLY_TIER +ALL_JOBS = QUEUE_TIER | NIGHTLY_TIER | SPARK_DEPRECATED | {"docs"} POLICY_CASES = [ # A manual run may exercise anything. ({"name": "workflow_dispatch"}, ALL_JOBS), # The merge queue is the authoritative gate: everything except the site - # deploy, which can only run once the commit is actually on main, and the - # deprecated Spark 3.4 suite, which no longer gates a merge. + # deploy, which can only run once the commit is actually on main, the + # nightly tier, and the deprecated Spark 3.4 suite, which no longer gates + # a merge. ({"name": "merge_group"}, QUEUE_TIER), + # The scheduled run is the nightly tier and nothing else. The queue already + # ran everything in QUEUE_TIER against the tree that is now main, so a + # queue job showing up here is a suite being paid for twice a day. + ({"name": "schedule"}, NIGHTLY_TIER), # Push to main is the site deploy plus the Linux build, which is there to # refresh main's actions/cache entries (see POLICY). `build_linux_full` # must stay out: it is what turns the lints and the test matrix back on, @@ -185,13 +205,13 @@ # opt-in suites stay off without their label. ({"name": "pull_request", "action": "opened", "labels": []}, PR_TIER), ({"name": "pull_request", "action": "synchronize", "labels": []}, PR_TIER), - # Spark 3.5 moved behind the queue; its label is the escape hatch. + # Spark 3.5 runs nightly; its label is the escape hatch. ( {"name": "pull_request", "action": "synchronize", "labels": ["run-spark-3.5-tests"]}, PR_TIER | {"spark_3_5"}, ), - # So did the macOS build and the benchmark compile check, each with its - # own label. Neither label pulls in the other. + # The macOS build and the benchmark compile check are queue-only, each + # with its own label. Neither label pulls in the other. ( {"name": "pull_request", "action": "synchronize", "labels": ["run-macos-tests"]}, PR_TIER | {"build_macos"}, @@ -229,10 +249,10 @@ }, {"build_macos"}, ), - # The linux-test matrix's non-default Spark profiles are queue-only with + # The linux-test matrix's non-default Spark profiles run nightly, with # their own label. On a pushed commit the label adds them to the PR tier's # Linux build call (`profiles: all`); on the `labeled` event alone it is - # the only output set, and ci.yml turns that into `profiles: queue-only` + # the only output set, and ci.yml turns that into `profiles: nightly` # so the default profile, which already ran at this commit, is not repeated. ( {"name": "pull_request", "action": "synchronize", "labels": ["run-all-spark-profiles"]}, @@ -422,11 +442,11 @@ def check_spark_sql_modules(): def check_linux_test_profiles(): - """`--profiles pr` and `--profiles queue-only` must partition `--profiles all`. + """`--profiles pr` and `--profiles nightly` must partition `--profiles all`. ci.yml maps `build_linux_full` and `build_linux_all_profiles` onto these three values. A profile in neither tier would never run anywhere; one in - both would run twice in the queue. The `pr` tier also has to be the + both would run twice on a labelled pull request. The `pr` tier also has to be the default build profile and nothing else, which is the whole reason the split exists. And the caller has to pass the input at all: its default is `all`, so a dropped `with:` line quietly puts every profile back on the @@ -438,14 +458,14 @@ def check_linux_test_profiles(): failures = [] names = lambda rows: [row["name"] for row in rows] everything = names(module.select("all")) - pr, queue_only = names(module.select("pr")), names(module.select("queue-only")) + pr, nightly = names(module.select("pr")), names(module.select("nightly")) if pr != ["Spark 4.1, JDK 17"]: failures.append(f"the pr tier must be the default build profile alone, got {pr}") - if not queue_only: - failures.append("the queue-only tier is empty (see PROFILES in dev/ci/linux-test-profiles.py)") - if sorted(pr + queue_only) != sorted(everything): + if not nightly: + failures.append("the nightly tier is empty (see PROFILES in dev/ci/linux-test-profiles.py)") + if sorted(pr + nightly) != sorted(everything): failures.append( - f"pr {pr} + queue-only {queue_only} does not partition all {everything} " + f"pr {pr} + nightly {nightly} does not partition all {everything} " f"(see PROFILES in dev/ci/linux-test-profiles.py)" ) if len(set(everything)) != len(everything): diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 5e21e2c68f..aee4283ff3 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -396,6 +396,7 @@ # # "pr" every pull request # "queue" the merge queue, i.e. a merge_group event +# "nightly" the scheduled run against main, once a day # "push" push to main # "label:" a pull request carrying that label # @@ -404,11 +405,23 @@ # requests or opt-in, never both -- and check-ci-config.py rejects a job that # lists both rather than letting the label quietly win. # -# Almost everything is "queue": the merge queue is the authoritative gate, and -# it tests the merge result rather than the PR head. "push" is reserved for -# work that can only happen once a commit is on main. Adding "push" back to a -# test job would make every merge run it twice, once in the queue and once -# after, which is the thing the queue was adopted to avoid. +# The merge queue is the authoritative gate: it tests the merge result rather +# than the PR head, and every "queue" job has to pass before a change lands. +# "nightly" is for the suites whose job is to catch a regression on a Spark or +# Iceberg version other than the default one. Those ran in the queue until +# mid-September 2026, when Spark 3.5, Spark 4.0 and Iceberg 1.8/1.9/1.10 were +# about 870 of the 1,900 runner-minutes a queue run cost, and the old Iceberg +# versions were the most common reason a queue run went red on a tree that was +# fine (issue #5870). A regression they catch is real but rare, and a day's +# delay in seeing it costs less than paying for the suites on every merge. The +# scheduled run tests main as it stands, so it skips itself when nothing has +# landed since the previous one, and a failure opens a `ci-nightly-failure` +# issue (see the `nightly_report` job in ci.yml). +# +# "push" is reserved for work that can only happen once a commit is on main. +# Adding "push" back to a test job would make every merge run it twice, once +# in the queue and once after, which is the thing the queue was adopted to +# avoid. POLICY = { # The one test job that also runs on push to main, and only because of # actions/cache scoping: a pull request can restore caches saved on its @@ -433,12 +446,13 @@ # The linux-test matrix's Spark profiles other than the default one. The # five profiles cost about the same each, roughly 2,300 runner-minutes a # day apiece on pull requests in mid-September 2026, and together they - # were three quarters of the Linux build. A pull request runs the Comet - # test suites against Spark 4.1 only; the queue runs all five. The - # lint-java matrix still compiles Spark 3.4/3.5/4.0 on every pull request, - # so what waits for the queue is runtime behaviour, not a shim that fails - # to build. ci.yml turns this output into the workflow's `profiles` input. - "build_linux_all_profiles": ["queue", "label:run-all-spark-profiles"], + # were three quarters of the Linux build. A pull request and the queue run + # the Comet test suites against Spark 4.1 only; the nightly run covers the + # other four. The lint-java matrix still compiles Spark 3.4/3.5/4.0 on + # every pull request, so what waits for the nightly is runtime behaviour, + # not a shim that fails to build. ci.yml turns this output into the + # workflow's `profiles` input. + "build_linux_all_profiles": ["nightly", "label:run-all-spark-profiles"], # macOS runners are the scarcest capacity we have, and the Linux build # already covers rustfmt and the Rust/JVM compile on every PR. The label # is for a change that touches platform-specific code. @@ -461,8 +475,11 @@ # demand -- the label on a pull request, or a workflow_dispatch -- so # anyone who wants to check a change against 3.4 still can. "spark_3_4": ["label:run-spark-3.4-tests"], - "spark_3_5": ["queue", "label:run-spark-3.5-tests"], - "spark_4_0": ["queue", "label:run-spark-4.0-tests"], + # Spark 4.1 is the default build profile and the one Spark SQL suite the + # queue runs; 3.5 and 4.0 run nightly, or on a pull request with their + # label. + "spark_3_5": ["nightly", "label:run-spark-3.5-tests"], + "spark_4_0": ["nightly", "label:run-spark-4.0-tests"], # No Spark SQL suite runs on a plain pull request. Spark 4.1 was the last # one in the PR tier, first whole (issue #5870 pulled the sql_hive shards # out) and then catalyst and sql_core alone. What changed is how often a @@ -478,12 +495,12 @@ "label:run-spark-4.1-tests", "label:run-spark-4.1-hive-tests", ], - # Same for Iceberg: 1.11 was the PR-tier version because it is the only - # Spark 4.1 coverage, and it now waits for the queue with the other three. - # One label opts a pull request into all four. - "iceberg_1_8": ["queue", "label:run-iceberg-tests"], - "iceberg_1_9": ["queue", "label:run-iceberg-tests"], - "iceberg_1_10": ["queue", "label:run-iceberg-tests"], + # Same shape for Iceberg: 1.11 is the only Spark 4.1 coverage, so it is + # the one Iceberg version the queue runs; the three older versions run + # nightly. One label opts a pull request into all four. + "iceberg_1_8": ["nightly", "label:run-iceberg-tests"], + "iceberg_1_9": ["nightly", "label:run-iceberg-tests"], + "iceberg_1_10": ["nightly", "label:run-iceberg-tests"], "iceberg_1_11": ["queue", "label:run-iceberg-tests"], } @@ -508,6 +525,8 @@ def event_allows(job, event): return "push" in tiers if name == "merge_group": return "queue" in tiers + if name == "schedule": + return "nightly" in tiers if name != "pull_request": return False @@ -587,12 +606,22 @@ def matches(patterns, files): if __name__ == "__main__": event = event_from_env() - # workflow_dispatch has no meaningful base to diff against, so the caller - # passes an empty list and every path filter is treated as matched. + # workflow_dispatch and schedule have no meaningful base to diff against, + # so the caller passes an empty list and every path filter is treated as + # matched. A dispatch runs everything; the nightly still goes through + # POLICY, so only the "nightly" tier runs. The caller also skips the + # nightly outright when nothing has landed since the previous one. if event["name"] == "workflow_dispatch": for name in FILTERS: print(f"{name}=true") sys.exit(0) + if event["name"] == "schedule": + # NIGHTLY_STALE is set by ci.yml when nothing has landed on main since + # the previous nightly; the whole tier is then skipped. + stale = os.environ.get("NIGHTLY_STALE", "") == "true" + for name in FILTERS: + print(f"{name}={'true' if not stale and event_allows(name, event) else 'false'}") + sys.exit(0) files_path = Path(sys.argv[1]) files = [line.strip() for line in files_path.read_text().splitlines() if line.strip()] for name, flag in compute(files, event).items(): diff --git a/dev/ci/linux-test-profiles.py b/dev/ci/linux-test-profiles.py index c2162aaf45..1638ccb45c 100644 --- a/dev/ci/linux-test-profiles.py +++ b/dev/ci/linux-test-profiles.py @@ -20,12 +20,12 @@ # # The five Spark profiles cost about the same each, and together they are # three quarters of what the Linux build spends on a pull request. Only one of -# them is the default build profile, so a pull request runs the Comet test -# suites against that one and the merge queue runs all five. A job-level `if:` -# cannot see `matrix`, so the selection has to happen before the matrix is -# expanded: the `lint` job runs this script and publishes the result as a job -# output that `linux-test` reads with `fromJSON`, the same way -# spark-sql-modules.py picks the Spark SQL shards. +# them is the default build profile, so a pull request and the merge queue run +# the Comet test suites against that one and the nightly run covers the other +# four. A job-level `if:` cannot see `matrix`, so the selection has to happen +# before the matrix is expanded: the `lint` job runs this script and publishes +# the result as a job output that `linux-test` reads with `fromJSON`, the same +# way spark-sql-modules.py picks the Spark SQL shards. # # `lint-java` keeps its own literal profile list. It compiles every profile it # can on every pull request (about five minutes each), which is what keeps a @@ -33,7 +33,7 @@ # runtime suites move behind it. # # Usage: -# linux-test-profiles.py --profiles all|pr|queue-only --github-output $GITHUB_OUTPUT +# linux-test-profiles.py --profiles all|pr|nightly --github-output $GITHUB_OUTPUT # linux-test-profiles.py --profiles pr (prints the matrix JSON) import argparse @@ -42,19 +42,19 @@ from pathlib import Path # `tier` is what --profiles selects on: "pr" rows run on every pull request -# and in the queue, "queue" rows only in the queue (or with the +# and in the queue, "nightly" rows only in the nightly run (or with the # `run-all-spark-profiles` label). The goal of the list is coverage of every # Java, Scala and Spark version without testing every combination. PROFILES = [ - {"name": "Spark 3.4, JDK 17, Scala 2.12", "java_version": "17", "maven_opts": "-Pspark-3.4 -Pscala-2.12", "tier": "queue"}, - {"name": "Spark 3.5, JDK 17, Scala 2.13", "java_version": "17", "maven_opts": "-Pspark-3.5 -Pscala-2.13", "tier": "queue"}, - {"name": "Spark 4.0, JDK 21", "java_version": "21", "maven_opts": "-Pspark-4.0", "tier": "queue"}, + {"name": "Spark 3.4, JDK 17, Scala 2.12", "java_version": "17", "maven_opts": "-Pspark-3.4 -Pscala-2.12", "tier": "nightly"}, + {"name": "Spark 3.5, JDK 17, Scala 2.13", "java_version": "17", "maven_opts": "-Pspark-3.5 -Pscala-2.13", "tier": "nightly"}, + {"name": "Spark 4.0, JDK 21", "java_version": "21", "maven_opts": "-Pspark-4.0", "tier": "nightly"}, # The default build profile, and the one a contributor builds locally. {"name": "Spark 4.1, JDK 17", "java_version": "17", "maven_opts": "-Pspark-4.1", "tier": "pr"}, - {"name": "Spark 4.2, JDK 17", "java_version": "17", "maven_opts": "-Pspark-4.2", "tier": "queue"}, + {"name": "Spark 4.2, JDK 17", "java_version": "17", "maven_opts": "-Pspark-4.2", "tier": "nightly"}, ] -SELECTORS = ("all", "pr", "queue-only") +SELECTORS = ("all", "pr", "nightly") def select(profiles): @@ -66,7 +66,7 @@ def select(profiles): elif profiles == "pr": rows = [row for row in PROFILES if row["tier"] == "pr"] else: - rows = [row for row in PROFILES if row["tier"] == "queue"] + rows = [row for row in PROFILES if row["tier"] == "nightly"] return [{key: value for key, value in row.items() if key != "tier"} for row in rows] @@ -75,7 +75,7 @@ def main(argv): parser.add_argument( "--profiles", default="all", - help="all, pr (the profiles every pull request runs) or queue-only (the ones it does not)", + help="all, pr (the profiles every pull request runs) or nightly (the ones it does not)", ) parser.add_argument("--github-output", type=Path, help="append matrix= to this $GITHUB_OUTPUT file") args = parser.parse_args(argv) diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 28d1ccc992..8eec23ba5d 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -25,32 +25,35 @@ how to opt a pull request into a suite the PR tier skips, and what to do when a mechanics behind the configuration are documented in [.github/workflows/README.md](https://github.com/apache/datafusion-comet/blob/main/.github/workflows/README.md). -## Two tiers +## Three tiers A single umbrella workflow, `.github/workflows/ci.yml`, orchestrates everything. It runs cheap preflight checks first (license headers, Markdown formatting, workflow linting, the CI config checks), computes which heavy jobs the changed files are relevant to, and fans out to those jobs. Which jobs run also depends on the event: -| Suite | Pull request | Merge queue | -| ------------------------------------------------- | ------------ | ----------- | -| Linux build, lint, Rust tests, TPC-H/TPC-DS | yes | yes | -| Comet test suites, Spark 4.1 | yes | yes | -| Comet test suites, Spark 3.4 / 3.5 / 4.0 / 4.2 | with label | yes | -| Spark SQL tests, Spark 4.1, catalyst and sql_core | with label | yes | -| Spark SQL tests, Spark 4.1, sql_hive | with label | yes | -| Iceberg Spark SQL tests, Iceberg 1.11 | with label | yes | -| macOS build and Comet test suites | with label | yes | -| Benchmark compile and lint check | with label | yes | -| Delta contrib build gate | with label | yes | -| PyArrow UDF tests, Spark 4.0 / 4.1 / 4.2 | with label | yes | -| Spark SQL tests, Spark 3.5 / 4.0 | with label | yes | -| Iceberg Spark SQL tests, Iceberg 1.8 / 1.9 / 1.10 | with label | yes | -| Spark SQL tests, Spark 3.4 | with label | no | +| Suite | Pull request | Merge queue | Nightly | +| ------------------------------------------------- | ------------ | ----------- | ------- | +| Linux build, lint, Rust tests, TPC-H/TPC-DS | yes | yes | no | +| Comet test suites, Spark 4.1 | yes | yes | no | +| Spark SQL tests, Spark 4.1, catalyst and sql_core | with label | yes | no | +| Spark SQL tests, Spark 4.1, sql_hive | with label | yes | no | +| Iceberg Spark SQL tests, Iceberg 1.11 | with label | yes | no | +| macOS build and Comet test suites | with label | yes | no | +| Benchmark compile and lint check | with label | yes | no | +| Delta contrib build gate | with label | yes | no | +| PyArrow UDF tests, Spark 4.0 / 4.1 / 4.2 | with label | yes | no | +| Comet test suites, Spark 3.4 / 3.5 / 4.0 / 4.2 | with label | no | yes | +| Spark SQL tests, Spark 3.5 / 4.0 | with label | no | yes | +| Iceberg Spark SQL tests, Iceberg 1.8 / 1.9 / 1.10 | with label | no | yes | +| Spark SQL tests, Spark 3.4 | with label | no | no | The **PR tier** is the fast feedback loop while a change is being iterated on. The **queue -tier** is the authoritative gate: everything the PR tier runs plus the remaining suites, evaluated -against the merge result rather than the pull request head. Nothing in the queue tier runs again +tier** is the authoritative gate: everything the PR tier runs plus the suites that must pass before +a change lands, evaluated against the merge result rather than the pull request head. The queue +runs one Spark version and one Iceberg version, the ones the default build profile targets. The +**nightly tier** runs the other Spark and Iceberg versions once a day against `main` as it stands; +see [Nightly runs](#nightly-runs) below. Nothing in the queue tier runs again on push to `main`, because the queue already tested the exact tree that landed. The one exception is the Linux build, which also runs on push so that the dependency caches on `main` stay fresh: a pull request can only restore caches saved on its own branch or on `main`, and the queue's @@ -100,9 +103,10 @@ Each suite outside the PR tier has a label that runs it on a pull request: | `run-spark-4.0-tests` | Spark SQL tests against Spark 4.0 | | `run-iceberg-tests` | Iceberg Spark SQL tests against every Iceberg version | -For every suite except Spark 3.4 the label only brings the run forward; the queue would have run -it anyway before the change landed. For Spark 3.4 the label is the only way the suite runs on a -pull request at all. +For a queue-tier suite the label only brings the run forward; the queue would have run it anyway +before the change landed. For a nightly-tier suite the label is what gets a verdict _before_ the +change lands: without it, the first run is the nightly after the merge. For Spark 3.4 the label is +the only way the suite runs at all. Apply a label from the pull request sidebar, or from the command line: @@ -115,13 +119,14 @@ executes only the suite the label gates; the PR tier already ran at that commit repeated. Its aggregate verdict is published as `Required Checks (label run)` rather than `Required Checks`, so it can be read alongside the commit run without replacing it. -For every suite except Spark 3.4, that separate name costs nothing: the merge queue runs the suite -again before the change lands, so a failure a label run surfaced still blocks the merge later. -Spark 3.4 has no queue run behind it. A red `Required Checks (label run)` leaves an earlier green -`Required Checks` in place and the pull request mergeable, so treat a label run as feedback to read, -not as a gate. For Spark 3.4 to count toward the required verdict the label has to already be on -the pull request when a commit is pushed — which is what the next push gives you, since the label -stays applied. +For a queue-tier suite, that separate name costs nothing: the merge queue runs the suite again +before the change lands, so a failure a label run surfaced still blocks the merge later. A +nightly-tier suite and Spark 3.4 have no queue run behind them. A red `Required Checks (label run)` +leaves an earlier green `Required Checks` in place and the pull request mergeable, so treat a label +run as feedback to read, not as a gate. For one of those suites to count toward the required +verdict the label has to already be on the pull request when a commit is pushed, which is what the +next push gives you, since the label stays applied. A nightly-tier regression that does slip +through is caught by the next nightly run and filed as an issue; a Spark 3.4 one is not. The label stays on the pull request, so every later push runs the suite as part of the normal PR run. Remove the label once it has served its purpose. To re-run the suite at the same commit, @@ -210,6 +215,30 @@ through these in order: A pull request evicted from the queue has to be queued again by hand. Merge when ready is not re-armed automatically. +## Nightly runs + +`ci.yml` also runs on a schedule, at 06:00 UTC every day, against `main` as it stands. That run +executes only the nightly tier: the Comet test suites against the Spark profiles other than 4.1, +the Spark SQL suites for Spark 3.5 and 4.0, and the Iceberg suites for 1.8, 1.9 and 1.10. The +queue already ran everything else against the same tree, so nothing in the queue tier is repeated. +The run skips itself when nothing has landed on `main` since the previous night. + +A red nightly has no pull request to show up on, so the run opens an issue labelled +`ci-nightly-failure` that links the run and lists the jobs that failed. If one of those issues is +already open, the run comments on it instead, so a failure that persists across several nights +stays in one place. When you pick up a nightly failure: + +1. **Find the commit.** The issue names the `main` commit the run tested. Compare it with the + previous green nightly (filter the Actions page by the `schedule` event) to narrow the change + that introduced it. +2. **Reproduce it on a pull request.** Open the fix as a pull request and apply the label for + the suite that failed, so the same suite runs against the fix before it lands. +3. **Close the issue** once the failure is understood, whether it was fixed, excluded, or found to + be infrastructure. The next red nightly opens a new issue. + +Dispatching `ci.yml` from the Actions page with **Run workflow** runs every tier, including the +nightly suites, if you need a result before the next scheduled run. + ## Reproducing a suite failure locally The Spark SQL suites outside the PR tier run Spark's own test suite against Comet, with the diff --git a/docs/source/contributor-guide/development.md b/docs/source/contributor-guide/development.md index f825cde080..b5752e0863 100644 --- a/docs/source/contributor-guide/development.md +++ b/docs/source/contributor-guide/development.md @@ -667,10 +667,10 @@ excluded and how to run it. Run a manual suite with: Only add a suite to that list with a good reason; the default is that a new suite runs in CI. -On a pull request the Linux build runs these suites against the default Spark profile (4.1) only; -the merge queue runs them against every Spark profile, and the macOS suites only run in the merge -queue by default. See [Continuous Integration](ci.md) for the two tiers and the labels that opt a -pull request into a queue-only suite. +On a pull request and in the merge queue the Linux build runs these suites against the default +Spark profile (4.1) only; the nightly run covers the other Spark profiles, and the macOS suites +only run in the merge queue by default. See [Continuous Integration](ci.md) for the three tiers +and the labels that opt a pull request into a queue-only or nightly suite. ### Pre-PR Summary diff --git a/docs/source/contributor-guide/iceberg-spark-tests.md b/docs/source/contributor-guide/iceberg-spark-tests.md index 8e45e43446..09374a041f 100644 --- a/docs/source/contributor-guide/iceberg-spark-tests.md +++ b/docs/source/contributor-guide/iceberg-spark-tests.md @@ -100,12 +100,12 @@ diff must be generated against its own tag. The `iceberg_spark_test_.yml` workflows apply these diffs and run the three Gradle targets above against each Iceberg version, all with Java 17. Iceberg 1.8.1 runs against Spark 3.4.3; Iceberg 1.9.1 and 1.10.0 -run against Spark 3.5.9; Iceberg 1.11.0 runs against Spark 4.1.3. All four versions run in the -merge queue, or earlier on a pull request labeled `run-iceberg-tests`; none runs on an unlabeled -pull request. All caller +run against Spark 3.5.9; Iceberg 1.11.0 runs against Spark 4.1.3. Iceberg 1.11.0 runs in the +merge queue; 1.8.1, 1.9.1 and 1.10.0 run once a night against `main`. All four run earlier on a +pull request labeled `run-iceberg-tests`; none runs on an unlabeled pull request. All caller workflows delegate to `iceberg_spark_test_reusable.yml`, which holds the build and test job logic. See [.github/workflows/README.md](https://github.com/apache/datafusion-comet/blob/main/.github/workflows/README.md) -for how the pull-request and merge-queue tiers differ. +for how the pull-request, merge-queue and nightly tiers differ. The core Spark test target runs in four independent workers. The workflow passes `dev/ci/iceberg-test-shards.gradle` as a Gradle init script: one worker runs the long @@ -132,4 +132,5 @@ and failure propagation. The fixture does not compile Spark or Iceberg. Apply the `run-iceberg-tests` label to a pull request whenever it touches the Iceberg scan or write path, reflection code (`org.apache.comet.iceberg.IcebergReflection`), or other logic whose behavior can differ across Iceberg versions. The Comet test suites in the Linux build do not exercise Iceberg's -own Spark tests, so without the label the first Iceberg verdict is the merge queue's. +own Spark tests, so without the label the first Iceberg 1.11 verdict is the merge queue's, and the +first verdict on the older Iceberg versions is the nightly run's, after the change has landed. diff --git a/docs/source/contributor-guide/spark-sql-tests.md b/docs/source/contributor-guide/spark-sql-tests.md index c4834bcff9..acfb8fcedc 100644 --- a/docs/source/contributor-guide/spark-sql-tests.md +++ b/docs/source/contributor-guide/spark-sql-tests.md @@ -169,9 +169,11 @@ git diff v3.5.6 > ../datafusion-comet/dev/diffs/3.5.6.diff ## Running Tests in CI The easiest way to run the tests is to open a pull request against Comet and let CI run them. Spark -3.5, 4.0 and 4.1 all run in the merge queue, or earlier on a pull request carrying the -`run-spark-3.5-tests` / `run-spark-4.0-tests` / `run-spark-4.1-tests` label. No Spark SQL suite -runs on an unlabeled pull request. +4.1 runs in the merge queue; Spark 3.5 and 4.0 run once a night against `main`. Each also runs +earlier on a pull request carrying the `run-spark-3.5-tests` / `run-spark-4.0-tests` / +`run-spark-4.1-tests` label. No Spark SQL suite runs on an unlabeled pull request. Apply the 3.5 or +4.0 label when a change could behave differently on those versions, since without it the first +verdict is the nightly run's, after the change has landed. Spark 3.4 is deprecated and is not run by the merge queue. It runs only when a pull request carries the `run-spark-3.4-tests` label, or when `ci.yml` is dispatched manually from the Actions page. From 079f764fd16fb694c799ee5ff41cfcccaf9ebbde Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 09:46:51 -0600 Subject: [PATCH 5/8] chore: trigger CI run [skip ci] From 0601ec0a329193646eb50c62f7dc544bb5ef5dcc Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 09:56:20 -0600 Subject: [PATCH 6/8] ci: route the nightly by diff and run only the test matrix under profiles nightly Diff the scheduled run against main's tip as of the previous tick instead of treating it as an event with no base. The nightly then goes through FILTERS like every other event: a quiet day or a docs-only day routes to nothing, and the NIGHTLY_STALE handshake and the schedule branch in compute-changes.py go away. With profiles nightly, pr_build_linux.yml now runs only lint, build-native and the linux-test matrix. The lints, Rust tests, Spark build and TPC runs already produced their verdict at the same commit, in the queue and push run for the nightly and in the PR tier for a label run. check-ci-config.py pins that guard the same way it pins cache-refresh-only, and the Cargo cache save is limited to the push run. Spell out QUEUE_TIER longhand, reject a POLICY entry that lists both queue and nightly, key the concurrency subgroup on the event name, reduce the profile selector to one comprehension, and trim repeated comments. --- .github/workflows/README.md | 25 +++--- .github/workflows/ci.yml | 43 ++++----- .github/workflows/pr_build_linux.yml | 41 ++++++--- dev/ci/check-ci-config.py | 130 ++++++++++++++++++++------- dev/ci/compute-changes.py | 32 +++---- dev/ci/linux-test-profiles.py | 7 +- docs/source/contributor-guide/ci.md | 11 +-- 7 files changed, 170 insertions(+), 119 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index c7fc5f174a..19af041790 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -212,13 +212,19 @@ makes the escape hatch look like it silently does nothing. ### Nightly tier `ci.yml` also fires on a `schedule` (06:00 UTC daily). On that event `changes` -has nothing to diff, so every path filter counts as matched and `POLICY` -selects the `nightly` tier alone: the queue already ran every `queue` job -against the tree that is now `main`, so repeating them would be paying twice. -The run skips itself when `HEAD` is older than the schedule interval (plus 30 -minutes of slack for scheduling jitter), so a quiet weekend does not re-test -the same tree three times. If a nightly fails for an infrastructure reason and -nothing lands the next day, dispatch `ci.yml` by hand. +diffs `main` against its tip as of the previous tick (24 hours back, plus 30 +minutes of slack for scheduling jitter), so the nightly is routed by the same +`FILTERS` as every other event and `POLICY` selects the `nightly` tier alone: +the queue already ran every `queue` job against the tree that is now `main`. +A quiet day diffs to nothing and runs nothing; a docs-only day runs nothing +either. If a nightly fails for an infrastructure reason and nothing lands the +next day, dispatch `ci.yml` by hand. + +With `profiles: nightly`, `pr_build_linux.yml` runs only `lint`, `build-native` +and the `linux-test` matrix; the lints, Rust tests, Spark build and TPC runs +carry `if: ${{ inputs.profiles != 'nightly' }}` because the queue and the push +run already produced those verdicts at the same commit. `check-ci-config.py` +pins that the same way it pins the cache-refresh guard. A red nightly has no pull request to appear on, and GitHub only emails a scheduled run's failure to whoever last touched the workflow file, so @@ -229,11 +235,6 @@ consecutive red nights accumulate in one issue. Closing the issue is how the failure is acknowledged; the next red night opens a new one. The label has to exist in repository settings, like the `run-*` labels above. -The nightly run is keyed on the same sha as the push-to-main run of the commit -at the tip of `main`, so the concurrency group gives it its own `nightly` -subgroup; otherwise a merge landing just before the schedule fired would have -its cache refresh cancelled. - ## Standalone workflows (not under the umbrella) These workflows have their own triggers because they fire on events the diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe3e942ae3..aa0fb7d8d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,10 +32,10 @@ # the default profile's. # schedule the nightly regression sweep of everything else: the Linux # test matrix against the other Spark profiles, Spark SQL on -# Spark 3.5 and 4.0, and Iceberg 1.8/1.9/1.10, against main -# as it stands. It skips itself when nothing has landed since -# the previous night, and a failure opens (or comments on) an -# issue labelled `ci-nightly-failure`. +# Spark 3.5 and 4.0, and Iceberg 1.8/1.9/1.10, routed by the +# same path filters over what landed on main since the +# previous tick. A failure opens (or comments on) an issue +# labelled `ci-nightly-failure`. # # Spark 3.4 is deprecated and sits outside all three tiers: it runs only when # a pull request carries `run-spark-3.4-tests`, or from a manual dispatch. @@ -55,13 +55,12 @@ name: CI # A `labeled` event (e.g. the run-spark-*-tests gates, or dependabot's automatic # `dependencies` label added ~1s after open) fires at the same commit as the # opened/synchronize run. Keying the group on the label name keeps labeled runs -# in their own subgroup so they never cancel the real commit run; opened and -# synchronize both map to `commit` so a new push still supersedes its predecessor. -# The nightly run is keyed on the same sha as the push-to-main run of the commit -# at the tip of main, so it gets its own subgroup too; otherwise a merge landing -# just before the schedule fires would have its cache refresh cancelled. +# in their own subgroup so they never cancel the real commit run. Every other +# run is keyed on its event name: opened, synchronize and reopened all map to +# `pull_request`, so a new push still supersedes its predecessor, while a +# scheduled run at the tip of main never cancels that commit's push run. concurrency: - group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event_name == 'schedule' && 'nightly' || github.event.action == 'labeled' && github.event.label.name || 'commit' }} + group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}-${{ github.event.action == 'labeled' && github.event.label.name || github.event_name }} cancel-in-progress: true on: @@ -221,19 +220,13 @@ jobs: # any gated job. : elif [[ "$EVENT_NAME" == "schedule" ]]; then - # Same: the nightly tests main as it stands, so there is nothing - # to diff and compute-changes.py runs the whole nightly tier. It - # skips when HEAD predates the previous night's run, so a quiet - # weekend does not re-test the same tree three times. The slack - # covers scheduling jitter; if a nightly fails for an - # infrastructure reason and nothing lands the next day, trigger - # ci.yml by hand instead. - head_ts=$(git log -1 --format=%ct) - age=$(( $(date +%s) - head_ts )) - if [[ "$age" -gt $(( 24 * 3600 + 1800 )) ]]; then - echo "HEAD is $(( age / 3600 ))h old; nothing has landed since the last nightly" - export NIGHTLY_STALE=true - fi + # The nightly's base is the tip of main as of the previous tick, + # with 30 minutes of slack for scheduling jitter. A quiet day + # diffs to nothing and runs nothing; a docs-only day routes to + # nothing too. If a nightly fails for an infrastructure reason + # and nothing lands the next day, dispatch ci.yml by hand. + prev=$(git rev-list -1 --before="24 hours 30 minutes ago" HEAD) + git diff --name-only "$prev"..HEAD > changed_files.txt elif [[ "$EVENT_NAME" == "pull_request" ]]; then git diff --name-only "$PR_BASE_SHA"..."$PR_HEAD_SHA" > changed_files.txt elif [[ "$EVENT_NAME" == "merge_group" ]]; then @@ -535,9 +528,7 @@ jobs: # # It sits downstream of the aggregator rather than of the nightly jobs # themselves so the job list here does not have to be kept in step with - # POLICY. It is exempt from `required_checks.needs` in - # dev/ci/check-ci-config.py for the same reason `docs` is: it is never part - # of a merge decision. + # POLICY. # --------------------------------------------------------------------------- nightly_report: name: Nightly failure report diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml index 77fdb2de00..58422f40ab 100644 --- a/.github/workflows/pr_build_linux.yml +++ b/.github/workflows/pr_build_linux.yml @@ -41,11 +41,18 @@ name: PR Build (Linux) # `run-all-spark-profiles` runs `all`. See the `build_linux_all_profiles` # comment in dev/ci/compute-changes.py. # -# Keeping both modes in this file is deliberate. The cache keys are the whole -# point of the push tier, and a separate cache-warming workflow would have to -# repeat every one of them; here they cannot drift apart. dev/ci/check-ci-config.py -# pins which jobs survive `cache-refresh-only`, so a job added without the guard -# cannot quietly put the full pipeline back on the push tier. +# `nightly` is also a third mode. Whenever ci.yml passes it, the default +# profile's pipeline has already run at this commit (in the queue and the push +# run for the scheduled event, in the PR tier for a label run), so only `lint`, +# `build-native` and the `linux-test` matrix run; every other job carries +# `if: ${{ inputs.profiles != 'nightly' }}`. +# +# Keeping all three modes in this file is deliberate. The cache keys are the +# whole point of the push tier, and a separate cache-warming workflow would +# have to repeat every one of them; here they cannot drift apart. +# dev/ci/check-ci-config.py pins which jobs survive `cache-refresh-only` and +# which run under `profiles: nightly`, so a job added without either guard +# cannot quietly put the full pipeline back on those tiers. on: workflow_call: inputs: @@ -108,7 +115,7 @@ jobs: # set, including the semantic rules, still runs in lint-java. scalafix-syntactic: name: Lint Scala (syntactic) - if: ${{ !inputs.cache-refresh-only }} + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v7 @@ -149,7 +156,7 @@ jobs: lint-java: needs: lint name: Lint Java (${{ matrix.profile.name }}) - if: ${{ !inputs.cache-refresh-only }} + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust @@ -229,7 +236,7 @@ jobs: build-spark-4-1: needs: lint name: Build Spark 4.1, JDK 17 - if: ${{ !inputs.cache-refresh-only }} + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust @@ -261,7 +268,7 @@ jobs: celeborn-reflection-compatibility: needs: lint name: Celeborn ${{ matrix.celeborn_version }} reflection compatibility - if: ${{ !inputs.cache-refresh-only }} + if: ${{ !inputs.cache-refresh-only && inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust @@ -351,7 +358,9 @@ jobs: - name: Save Cargo cache uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' + # The push run is the cache warmer (see the header); a scheduled run + # at the same sha would only re-archive an entry that already exists. + if: github.event_name == 'push' with: path: | ~/.cargo/registry @@ -368,6 +377,7 @@ jobs: linux-test-rust: needs: lint name: ubuntu-latest/rust-test + if: ${{ inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust @@ -397,7 +407,9 @@ jobs: - name: Save Cargo cache uses: actions/cache/save@v6 - if: github.ref == 'refs/heads/main' + # The push run is the cache warmer (see the header); a scheduled run + # at the same sha would only re-archive an entry that already exists. + if: github.event_name == 'push' with: path: | ~/.cargo/registry @@ -415,9 +427,8 @@ jobs: # The Spark profiles live in dev/ci/linux-test-profiles.py and are # selected by the `profiles` input: the default build profile on a # pull request and in the merge queue, the other four in the nightly - # run. The goal of the full list - # is coverage of every Java, Scala and Spark version without testing - # every combination. + # run. The goal of the full list is coverage of every Java, Scala and + # Spark version without testing every combination. profile: ${{ fromJSON(needs.lint.outputs.profile-matrix) }} # Suites are grouped by functional area into balanced buckets so that no test # job runs much longer than ~23 min. See @@ -613,6 +624,7 @@ jobs: verify-benchmark-results-tpch: needs: build-native name: Verify TPC-H Results + if: ${{ inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust @@ -677,6 +689,7 @@ jobs: verify-benchmark-results-tpcds: needs: build-native name: Verify TPC-DS Results + if: ${{ inputs.profiles != 'nightly' }} runs-on: ubuntu-24.04 container: image: amd64/rust diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index ca43442c59..751a644bba 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -150,28 +150,20 @@ # The PR tier is the Linux build and nothing else. Every Spark SQL and Iceberg # suite waits for the queue or the nightly run, or for its label. PR_TIER = {"build_linux", "build_linux_full"} -SPARK_OPT_IN = {"spark_3_5", "spark_4_0", "spark_4_1", "spark_4_1_hive"} -# Spark 3.4 is deprecated and sits outside the queue and nightly tiers -# entirely: a label on a pull request, or a workflow_dispatch, and nothing -# else. Keeping it in its own set is what makes the `merge_group` and -# `schedule` cases below assert its absence rather than quietly accept it -# coming back. -SPARK_DEPRECATED = {"spark_3_4"} -ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"} -# `build_linux_all_profiles` is the linux-test matrix's non-default Spark -# profiles: part of the Linux build's call, not a job of its own. -BUILD_OPT_IN = { +# The queue adds one Spark version (4.1, the default profile) and one Iceberg +# version (1.11, the only Spark 4.1 coverage), plus the build-level gates. +QUEUE_TIER = PR_TIER | { + "spark_4_1", + "spark_4_1_hive", + "iceberg_1_11", "build_macos", "benchmark", - "build_linux_all_profiles", "delta_gate", "pyarrow_udf", } -# The queue runs one Spark version (4.1, the default profile) and one Iceberg -# version (1.11, the only Spark 4.1 coverage). Every other Spark and Iceberg -# version, and the linux-test matrix's other Spark profiles, run once a night -# against main instead. Spelled out as the set of jobs the queue must *not* -# run, so a suite drifting back into the queue fails the `merge_group` case. +# Every other Spark and Iceberg version, and the linux-test matrix's other +# Spark profiles (`build_linux_all_profiles`, part of the Linux build's call +# rather than a job of its own), run once a night against main instead. NIGHTLY_TIER = { "spark_3_5", "spark_4_0", @@ -180,8 +172,17 @@ "iceberg_1_10", "build_linux_all_profiles", } -QUEUE_TIER = (PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN) - NIGHTLY_TIER +# Spark 3.4 is deprecated and sits outside the queue and nightly tiers +# entirely: a label on a pull request, or a workflow_dispatch, and nothing +# else. Keeping it in its own set is what makes the `merge_group` and +# `schedule` cases below assert its absence rather than quietly accept it +# coming back. +SPARK_DEPRECATED = {"spark_3_4"} +# One label opts a pull request into every Iceberg version, whichever tier +# each sits in. +ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10", "iceberg_1_11"} ALL_JOBS = QUEUE_TIER | NIGHTLY_TIER | SPARK_DEPRECATED | {"docs"} +assert not QUEUE_TIER & NIGHTLY_TIER, "a job is queue or nightly, never both" POLICY_CASES = [ # A manual run may exercise anything. @@ -400,6 +401,33 @@ # indented further, and those are expected rather than a reason to exempt the # whole job. CACHE_REFRESH_GUARD = re.compile(r"^ if:.*!\s*inputs\.cache-refresh-only") +# The same file's third mode: with `profiles: nightly` only the jobs the +# linux-test matrix needs run. See check_nightly_scope. +NIGHTLY_JOBS = { + "lint": "publishes the profile matrix that linux-test reads", + "build-native": "builds the native library the matrix loads", + "linux-test": "the matrix itself", +} +NIGHTLY_GUARD = re.compile(r"^ if:.*inputs\.profiles\s*!=\s*'nightly'") + + +def guarded_jobs(path, guard): + """Return (job ids in `path`, the subset whose job-level `if:` matches `guard`).""" + jobs, guarded, job, in_jobs = [], set(), None, False + for line in path.read_text(encoding="utf-8").splitlines(): + if line.startswith("jobs:"): + in_jobs = True + continue + if not in_jobs or line.lstrip().startswith("#"): + continue + match = JOB_KEY.match(line) + if match: + job = match.group(1) + jobs.append(job) + continue + if job and guard.match(line): + guarded.add(job) + return jobs, guarded CACHE_REFRESH_INPUT = re.compile(r"^\s+cache-refresh-only:\s*\$\{\{") # `profiles:` is passed as a folded scalar (`>-`) whose expression sits on the # next line, so match the key alone. @@ -446,8 +474,8 @@ def check_linux_test_profiles(): ci.yml maps `build_linux_full` and `build_linux_all_profiles` onto these three values. A profile in neither tier would never run anywhere; one in - both would run twice on a labelled pull request. The `pr` tier also has to be the - default build profile and nothing else, which is the whole reason the + both would run twice on a labelled pull request. The `pr` tier also has + to be the default build profile and nothing else, which is the reason the split exists. And the caller has to pass the input at all: its default is `all`, so a dropped `with:` line quietly puts every profile back on the pull request tier. @@ -534,6 +562,13 @@ def check_event_policy(): f"mutually exclusive; drop 'pr' if the job is opt-in, or drop " f"the label if it should run on every pull request" ) + # The nightly exists so the queue does not pay for these suites; a job + # in both tiers would run twice a day for one verdict. + if "queue" in tiers and "nightly" in tiers: + failures.append( + f"{job}: POLICY lists both 'queue' and 'nightly'. The queue " + f"already tested the tree the nightly runs against, so pick one" + ) for failure in failures: print(f"event policy: {failure}") return not failures @@ -813,20 +848,7 @@ def check_cache_refresh_scope(): minutes a push, which is what this check exists to notice. """ failures = [] - jobs, guarded, job, in_jobs = [], set(), None, False - for line in CACHE_REFRESH_WORKFLOW.read_text(encoding="utf-8").splitlines(): - if line.startswith("jobs:"): - in_jobs = True - continue - if not in_jobs or line.lstrip().startswith("#"): - continue - match = JOB_KEY.match(line) - if match: - job = match.group(1) - jobs.append(job) - continue - if job and CACHE_REFRESH_GUARD.match(line): - guarded.add(job) + jobs, guarded = guarded_jobs(CACHE_REFRESH_WORKFLOW, CACHE_REFRESH_GUARD) for stale in sorted(set(CACHE_REFRESH_JOBS) - set(jobs)): failures.append( @@ -866,6 +888,45 @@ def check_cache_refresh_scope(): return not failures +def check_nightly_scope(): + """With `profiles: nightly`, pr_build_linux.yml runs the test matrix alone. + + That input value means the default-profile pipeline already ran at this + commit: in the queue and the push run for the nightly, in the PR tier for + a `run-all-spark-profiles` label run. The lints, the Rust tests, the + Spark build and the TPC-H/TPC-DS runs would repeat a verdict, so every + job other than the three the matrix needs carries + `if: ${{ inputs.profiles != 'nightly' }}`. Silent when broken, like the + cache-refresh guard: the nightly just costs 100-odd runner-minutes more. + """ + failures = [] + jobs, guarded = guarded_jobs(CACHE_REFRESH_WORKFLOW, NIGHTLY_GUARD) + for stale in sorted(set(NIGHTLY_JOBS) - set(jobs)): + failures.append( + f"NIGHTLY_JOBS names `{stale}`, which no longer exists in " + f"{CACHE_REFRESH_WORKFLOW}; drop it here, or restore the job" + ) + for name in jobs: + if name in NIGHTLY_JOBS and name in guarded: + failures.append( + f"{CACHE_REFRESH_WORKFLOW}: job `{name}` is listed in " + f"NIGHTLY_JOBS ({NIGHTLY_JOBS[name]}) but carries the " + f"`profiles != 'nightly'` guard, so the nightly test matrix " + f"cannot run" + ) + if name not in NIGHTLY_JOBS and name not in guarded: + failures.append( + f"{CACHE_REFRESH_WORKFLOW}: job `{name}` has no " + f"`if: ${{{{ inputs.profiles != 'nightly' }}}}`, so it repeats " + f"on every nightly a verdict the queue already produced. Add " + f"the guard, or add the job to NIGHTLY_JOBS with the reason " + f"the test matrix needs it" + ) + for failure in failures: + print(f"nightly scope: {failure}") + return not failures + + if __name__ == "__main__": ok = check_change_filters() ok = check_event_policy() and ok @@ -875,6 +936,7 @@ def check_cache_refresh_scope(): ok = check_local_actions_have_checkout() and ok ok = check_required_checks() and ok ok = check_cache_refresh_scope() and ok + ok = check_nightly_scope() and ok if not ok: sys.exit(1) print("CI config checks passed") diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index aee4283ff3..8779706601 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -407,16 +407,14 @@ # # The merge queue is the authoritative gate: it tests the merge result rather # than the PR head, and every "queue" job has to pass before a change lands. -# "nightly" is for the suites whose job is to catch a regression on a Spark or -# Iceberg version other than the default one. Those ran in the queue until -# mid-September 2026, when Spark 3.5, Spark 4.0 and Iceberg 1.8/1.9/1.10 were -# about 870 of the 1,900 runner-minutes a queue run cost, and the old Iceberg -# versions were the most common reason a queue run went red on a tree that was -# fine (issue #5870). A regression they catch is real but rare, and a day's -# delay in seeing it costs less than paying for the suites on every merge. The -# scheduled run tests main as it stands, so it skips itself when nothing has -# landed since the previous one, and a failure opens a `ci-nightly-failure` -# issue (see the `nightly_report` job in ci.yml). +# "nightly" is for the suites that catch a regression on a Spark or Iceberg +# version other than the default one: about 870 of the 1,900 runner-minutes a +# queue run cost in September 2026, and the most common reason a queue run +# went red on a good tree (issue #5870). A regression there is real but rare, +# and a day's delay in seeing it costs less than running the suites on every +# merge. The scheduled run diffs main against the previous tick and routes +# through FILTERS like any other event. A job is "queue" or "nightly", never +# both; check-ci-config.py enforces that. # # "push" is reserved for work that can only happen once a commit is on main. # Adding "push" back to a test job would make every merge run it twice, once @@ -606,22 +604,12 @@ def matches(patterns, files): if __name__ == "__main__": event = event_from_env() - # workflow_dispatch and schedule have no meaningful base to diff against, - # so the caller passes an empty list and every path filter is treated as - # matched. A dispatch runs everything; the nightly still goes through - # POLICY, so only the "nightly" tier runs. The caller also skips the - # nightly outright when nothing has landed since the previous one. + # workflow_dispatch has no meaningful base to diff against, so the caller + # passes an empty list and every path filter is treated as matched. if event["name"] == "workflow_dispatch": for name in FILTERS: print(f"{name}=true") sys.exit(0) - if event["name"] == "schedule": - # NIGHTLY_STALE is set by ci.yml when nothing has landed on main since - # the previous nightly; the whole tier is then skipped. - stale = os.environ.get("NIGHTLY_STALE", "") == "true" - for name in FILTERS: - print(f"{name}={'true' if not stale and event_allows(name, event) else 'false'}") - sys.exit(0) files_path = Path(sys.argv[1]) files = [line.strip() for line in files_path.read_text().splitlines() if line.strip()] for name, flag in compute(files, event).items(): diff --git a/dev/ci/linux-test-profiles.py b/dev/ci/linux-test-profiles.py index 1638ccb45c..741ef7d3b1 100644 --- a/dev/ci/linux-test-profiles.py +++ b/dev/ci/linux-test-profiles.py @@ -61,12 +61,7 @@ def select(profiles): """Return the matrix rows for a --profiles value, or raise ValueError.""" if profiles not in SELECTORS: raise ValueError(f"--profiles must be one of {', '.join(SELECTORS)}, got {profiles!r}") - if profiles == "all": - rows = PROFILES - elif profiles == "pr": - rows = [row for row in PROFILES if row["tier"] == "pr"] - else: - rows = [row for row in PROFILES if row["tier"] == "nightly"] + rows = [row for row in PROFILES if profiles == "all" or row["tier"] == profiles] return [{key: value for key, value in row.items() if key != "tier"} for row in rows] diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 8eec23ba5d..caed0793ce 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -217,11 +217,12 @@ re-armed automatically. ## Nightly runs -`ci.yml` also runs on a schedule, at 06:00 UTC every day, against `main` as it stands. That run -executes only the nightly tier: the Comet test suites against the Spark profiles other than 4.1, -the Spark SQL suites for Spark 3.5 and 4.0, and the Iceberg suites for 1.8, 1.9 and 1.10. The -queue already ran everything else against the same tree, so nothing in the queue tier is repeated. -The run skips itself when nothing has landed on `main` since the previous night. +`ci.yml` also runs on a schedule, at 06:00 UTC every day, against what landed on `main` since the +previous run. That run executes only the nightly tier: the Comet test suites against the Spark +profiles other than 4.1, the Spark SQL suites for Spark 3.5 and 4.0, and the Iceberg suites for +1.8, 1.9 and 1.10, each only when the day's changes touched files it covers. The queue already ran +everything else against the same tree, so nothing in the queue tier is repeated. A day with no +merges, or with only documentation changes, runs nothing. A red nightly has no pull request to show up on, so the run opens an issue labelled `ci-nightly-failure` that links the run and lists the jobs that failed. If one of those issues is From 8533d0cf08cdf813e20ba708c0a4df9f07b4d73b Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 10:13:41 -0600 Subject: [PATCH 7/8] ci: base the nightly diff on the last successful scheduled run [skip ci] Look up the head sha of the most recent successful scheduled ci.yml run through the Actions API and diff main against it, so every commit is covered by exactly one nightly and a red night keeps its commits in scope until a green one supersedes it. The 24h30m window stays as the fallback for the first run, an API error, or a base that is no longer on main. --- .github/workflows/README.md | 17 +++++--- .github/workflows/ci.yml | 29 +++++++++---- dev/ci/nightly-base.py | 66 +++++++++++++++++++++++++++++ docs/source/contributor-guide/ci.md | 8 ++-- 4 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 dev/ci/nightly-base.py diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 19af041790..1875997142 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -212,13 +212,16 @@ makes the escape hatch look like it silently does nothing. ### Nightly tier `ci.yml` also fires on a `schedule` (06:00 UTC daily). On that event `changes` -diffs `main` against its tip as of the previous tick (24 hours back, plus 30 -minutes of slack for scheduling jitter), so the nightly is routed by the same -`FILTERS` as every other event and `POLICY` selects the `nightly` tier alone: -the queue already ran every `queue` job against the tree that is now `main`. -A quiet day diffs to nothing and runs nothing; a docs-only day runs nothing -either. If a nightly fails for an infrastructure reason and nothing lands the -next day, dispatch `ci.yml` by hand. +diffs `main` against the commit the last successful scheduled run tested +(`dev/ci/nightly-base.py` looks it up through the Actions API), so the nightly +is routed by the same `FILTERS` as every other event and `POLICY` selects the +`nightly` tier alone: the queue already ran every `queue` job against the tree +that is now `main`. Every commit is covered exactly once, and a red nightly +keeps its commits in scope until a green one supersedes it. When there is no +such run, the API is unreachable, or the base is no longer on `main`, the base +falls back to the tip as of the previous tick, 24 hours back plus 30 minutes +of slack for scheduling jitter. A quiet day diffs to nothing and runs nothing; +a docs-only day runs nothing either. With `profiles: nightly`, `pr_build_linux.yml` runs only `lint`, `build-native` and the `linux-test` matrix; the lints, Rust tests, Spark build and TPC runs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa0fb7d8d5..404c72106f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,8 +33,8 @@ # schedule the nightly regression sweep of everything else: the Linux # test matrix against the other Spark profiles, Spark SQL on # Spark 3.5 and 4.0, and Iceberg 1.8/1.9/1.10, routed by the -# same path filters over what landed on main since the -# previous tick. A failure opens (or comments on) an issue +# same path filters over what landed on main since the last +# green nightly. A failure opens (or comments on) an issue # labelled `ci-nightly-failure`. # # Spark 3.4 is deprecated and sits outside all three tiers: it runs only when @@ -172,6 +172,11 @@ jobs: name: Detect changes needs: preflight runs-on: ubuntu-slim + # `actions: read` is for dev/ci/nightly-base.py, which lists previous + # scheduled runs to pick the nightly's diff base. + permissions: + actions: read + contents: read outputs: build_linux: ${{ steps.compute.outputs.build_linux }} build_linux_full: ${{ steps.compute.outputs.build_linux_full }} @@ -211,6 +216,7 @@ jobs: MQ_HEAD_SHA: ${{ github.event.merge_group.head_sha }} PUSH_BEFORE: ${{ github.event.before }} PUSH_AFTER: ${{ github.sha }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail : > changed_files.txt @@ -220,12 +226,19 @@ jobs: # any gated job. : elif [[ "$EVENT_NAME" == "schedule" ]]; then - # The nightly's base is the tip of main as of the previous tick, - # with 30 minutes of slack for scheduling jitter. A quiet day - # diffs to nothing and runs nothing; a docs-only day routes to - # nothing too. If a nightly fails for an infrastructure reason - # and nothing lands the next day, dispatch ci.yml by hand. - prev=$(git rev-list -1 --before="24 hours 30 minutes ago" HEAD) + # The nightly's base is the commit the last successful scheduled + # run tested, so everything that landed since is covered exactly + # once and a red nightly keeps its commits in scope until a green + # one supersedes it. Without such a run (the first nightly, an API + # error, or a base no longer on main) fall back to the tip as of + # the previous tick, with 30 minutes of slack for scheduling + # jitter. A quiet day diffs to nothing and runs nothing; so does + # a docs-only day. + prev=$(python3 dev/ci/nightly-base.py) + if [[ -z "$prev" ]] || ! git merge-base --is-ancestor "$prev" HEAD 2>/dev/null; then + prev=$(git rev-list -1 --before="24 hours 30 minutes ago" HEAD) + fi + echo "Nightly base: $prev" git diff --name-only "$prev"..HEAD > changed_files.txt elif [[ "$EVENT_NAME" == "pull_request" ]]; then git diff --name-only "$PR_BASE_SHA"..."$PR_HEAD_SHA" > changed_files.txt diff --git a/dev/ci/nightly-base.py b/dev/ci/nightly-base.py new file mode 100644 index 0000000000..b51012fe5d --- /dev/null +++ b/dev/ci/nightly-base.py @@ -0,0 +1,66 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Print the head sha of the most recent successful scheduled run of ci.yml, +# or nothing. +# +# The nightly diffs main against that commit, so every commit that landed +# since the last green nightly is covered exactly once, and a red nightly +# keeps the regressing commits in scope until a green one supersedes it. The +# `changes` job in ci.yml falls back to a time-based base when this prints +# nothing: the first scheduled run, or an API error, which is reported as a +# warning rather than failing the run. +# +# Needs GITHUB_TOKEN with `actions: read`, plus the GITHUB_REPOSITORY and +# GITHUB_API_URL variables every job has. + +import json +import os +import sys +import urllib.error +import urllib.request + + +def main(): + repo = os.environ["GITHUB_REPOSITORY"] + api = os.environ.get("GITHUB_API_URL", "https://api.github.com") + url = f"{api}/repos/{repo}/actions/workflows/ci.yml/runs?event=schedule&status=success&per_page=1" + request = urllib.request.Request( + url, + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {os.environ['GITHUB_TOKEN']}", + "X-GitHub-Api-Version": "2022-11-28", + }, + ) + try: + with urllib.request.urlopen(request, timeout=30) as response: + runs = json.load(response)["workflow_runs"] + except (urllib.error.URLError, OSError, KeyError, ValueError) as e: + # stderr, so the caller's command substitution stays empty. + print( + f"::warning::could not list previous scheduled runs ({e}); using the time-based base", + file=sys.stderr, + ) + return 0 + if runs: + print(runs[0]["head_sha"]) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index caed0793ce..7892b18d61 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -218,7 +218,7 @@ re-armed automatically. ## Nightly runs `ci.yml` also runs on a schedule, at 06:00 UTC every day, against what landed on `main` since the -previous run. That run executes only the nightly tier: the Comet test suites against the Spark +last successful scheduled run. That run executes only the nightly tier: the Comet test suites against the Spark profiles other than 4.1, the Spark SQL suites for Spark 3.5 and 4.0, and the Iceberg suites for 1.8, 1.9 and 1.10, each only when the day's changes touched files it covers. The queue already ran everything else against the same tree, so nothing in the queue tier is repeated. A day with no @@ -229,9 +229,9 @@ A red nightly has no pull request to show up on, so the run opens an issue label already open, the run comments on it instead, so a failure that persists across several nights stays in one place. When you pick up a nightly failure: -1. **Find the commit.** The issue names the `main` commit the run tested. Compare it with the - previous green nightly (filter the Actions page by the `schedule` event) to narrow the change - that introduced it. +1. **Find the commit.** The issue names the `main` commit the run tested, and the run's + `Detect changes` log names the base it diffed against, which is the commit the previous green + nightly tested. The regression is in that range. 2. **Reproduce it on a pull request.** Open the fix as a pull request and apply the label for the suite that failed, so the same suite runs against the fix before it lands. 3. **Close the issue** once the failure is understood, whether it was fixed, excluded, or found to From 01d000fc6b70ce94f8988c82712577b215ac418b Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 11:19:44 -0600 Subject: [PATCH 8/8] ci: run the whole nightly tier when the base is unknown The scheduled run's base is the commit the last successful scheduled run tested. When that lookup yields nothing -- the first nightly, an Actions API error, or a base no longer on main -- it fell back to the tip as of 24 hours ago. That window can silently drop coverage: if the last green nightly was three nights ago and an uncovered source change landed two nights ago, a one-day window starts after that change, so its suites are skipped, the run goes green, and the green head becomes tomorrow's base. Fall back to listing the whole tree instead, the same way the first push to a branch does, so POLICY selects the entire nightly tier and a green run is a base that can be trusted. check-ci-config.py pins the shape of the schedule case, since the failure it prevents is silent. --- .github/workflows/README.md | 15 ++++-- .github/workflows/ci.yml | 26 ++++++---- dev/ci/check-ci-config.py | 79 +++++++++++++++++++++++++++++ dev/ci/compute-changes.py | 6 +-- dev/ci/nightly-base.py | 15 ++++-- docs/source/contributor-guide/ci.md | 4 ++ 6 files changed, 123 insertions(+), 22 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 1875997142..0f1f93fe94 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -217,11 +217,16 @@ diffs `main` against the commit the last successful scheduled run tested is routed by the same `FILTERS` as every other event and `POLICY` selects the `nightly` tier alone: the queue already ran every `queue` job against the tree that is now `main`. Every commit is covered exactly once, and a red nightly -keeps its commits in scope until a green one supersedes it. When there is no -such run, the API is unreachable, or the base is no longer on `main`, the base -falls back to the tip as of the previous tick, 24 hours back plus 30 minutes -of slack for scheduling jitter. A quiet day diffs to nothing and runs nothing; -a docs-only day runs nothing either. +keeps its commits in scope until a green one supersedes it. A quiet day diffs +to nothing and runs nothing; a docs-only day runs nothing either. + +When there is no such run, the API is unreachable, or the base is no longer on +`main`, there is no base to diff against and the run treats every tracked file +as changed, so the whole nightly tier runs. Guessing a narrower base — a fixed +time window, say — would be worse than useless: a window that starts after a +commit no nightly has covered yet skips the suites that commit needs, lets the +run go green, and then hands that green head to `nightly-base.py` as tomorrow's +base, so the coverage is dropped for good. With `profiles: nightly`, `pr_build_linux.yml` runs only `lint`, `build-native` and the `linux-test` matrix; the lints, Rust tests, Spark build and TPC runs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 404c72106f..d5bdde5aa6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -229,17 +229,25 @@ jobs: # The nightly's base is the commit the last successful scheduled # run tested, so everything that landed since is covered exactly # once and a red nightly keeps its commits in scope until a green - # one supersedes it. Without such a run (the first nightly, an API - # error, or a base no longer on main) fall back to the tip as of - # the previous tick, with 30 minutes of slack for scheduling - # jitter. A quiet day diffs to nothing and runs nothing; so does - # a docs-only day. + # one supersedes it. A quiet day diffs to nothing and runs + # nothing; so does a docs-only day. + # + # Without a base -- the first nightly, an API error, or a base no + # longer on main -- run the whole tier by treating every tracked + # file as changed, the same way the first push to a branch does + # below. Guessing a narrower base would be worse than running + # nothing: a window that starts after a commit no nightly has + # covered yet skips the suites that commit needs, goes green + # anyway, and then becomes the base for tomorrow, dropping that + # coverage for good. prev=$(python3 dev/ci/nightly-base.py) - if [[ -z "$prev" ]] || ! git merge-base --is-ancestor "$prev" HEAD 2>/dev/null; then - prev=$(git rev-list -1 --before="24 hours 30 minutes ago" HEAD) + if [[ -n "$prev" ]] && git merge-base --is-ancestor "$prev" HEAD 2>/dev/null; then + echo "Nightly base: $prev" + git diff --name-only "$prev"..HEAD > changed_files.txt + else + echo "Nightly base unknown; running the whole nightly tier" + git ls-tree -r --name-only HEAD > changed_files.txt fi - echo "Nightly base: $prev" - git diff --name-only "$prev"..HEAD > changed_files.txt elif [[ "$EVENT_NAME" == "pull_request" ]]; then git diff --name-only "$PR_BASE_SHA"..."$PR_HEAD_SHA" > changed_files.txt elif [[ "$EVENT_NAME" == "merge_group" ]]; then diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 751a644bba..b271bc60f6 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -409,6 +409,12 @@ "linux-test": "the matrix itself", } NIGHTLY_GUARD = re.compile(r"^ if:.*inputs\.profiles\s*!=\s*'nightly'") +# The `schedule` case in ci.yml's `Detect changes` script, and what has to be +# in it. See check_nightly_base_fallback. +CI_WORKFLOW = WORKFLOWS / "ci.yml" +SCHEDULE_BRANCH = re.compile(r'^(\s+)elif \[\[ "\$EVENT_NAME" == "schedule" \]\]; then\s*$') +# Any base derived from a clock rather than from the previous run. +DATE_BASED_BASE = re.compile(r"--(before|since|after|until)\b") def guarded_jobs(path, guard): @@ -927,6 +933,78 @@ def check_nightly_scope(): return not failures +def schedule_branch(): + """The body of the `schedule` case in ci.yml's `Detect changes` script. + + Ends at the next branch keyword indented the same as the `elif` itself, so + the `if`/`else` nested inside the case stays part of the body. + """ + body, indent = [], None + for line in CI_WORKFLOW.read_text(encoding="utf-8").splitlines(): + if indent is None: + match = SCHEDULE_BRANCH.match(line) + if match: + indent = match.group(1) + continue + if re.match(rf"^{indent}(elif|else|fi)\b", line): + break + body.append(line) + return body + + +def check_nightly_base_fallback(): + """The nightly must run everything when it has no base to diff against. + + `dev/ci/nightly-base.py` returns the commit the last successful scheduled + run tested, and the nightly diffs `main` against it. When it returns + nothing -- the first nightly, an Actions API error, or a base that has + left `main` -- the only safe base is no base at all: list the whole tree + and let POLICY narrow it to the nightly tier. + + Guessing a narrower base is silently destructive, which is why it is + pinned here. Say the last green nightly was three nights ago, a source + change landed two nights ago and no nightly has covered it, and only docs + have landed since. A one-day window starts after that source change, so + its suites are skipped, the run goes green, and `nightly-base.py` then + hands that green head out as tomorrow's base. The coverage is gone, the + run that dropped it was green, and nothing says so. + """ + failures = [] + body = schedule_branch() + if not body: + failures.append( + f"{CI_WORKFLOW}: no `schedule` case in the `Detect changes` script " + f"(SCHEDULE_BRANCH no longer matches); the nightly is unrouted" + ) + code = [line for line in body if not line.lstrip().startswith("#")] + text = "\n".join(code) + if "dev/ci/nightly-base.py" not in text: + failures.append( + f"{CI_WORKFLOW}: the `schedule` case does not call " + f"dev/ci/nightly-base.py, so the nightly is not based on the last " + f"successful scheduled run and commits can be covered twice or not " + f"at all" + ) + if "git ls-tree -r --name-only HEAD" not in text: + failures.append( + f"{CI_WORKFLOW}: the `schedule` case has no " + f"`git ls-tree -r --name-only HEAD` fallback, so a nightly with no " + f"base does not run the whole tier" + ) + for line in code: + if DATE_BASED_BASE.search(line): + failures.append( + f"{CI_WORKFLOW}: the `schedule` case derives a base from the " + f"clock (`{line.strip()}`). A window that starts after a commit " + f"no nightly has covered yet skips that commit's suites, goes " + f"green, and becomes tomorrow's base. Fall back to the whole " + f"tree instead" + ) + for failure in failures: + print(f"nightly base: {failure}") + return not failures + + if __name__ == "__main__": ok = check_change_filters() ok = check_event_policy() and ok @@ -937,6 +1015,7 @@ def check_nightly_scope(): ok = check_required_checks() and ok ok = check_cache_refresh_scope() and ok ok = check_nightly_scope() and ok + ok = check_nightly_base_fallback() and ok if not ok: sys.exit(1) print("CI config checks passed") diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 8779706601..799b791624 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -412,9 +412,9 @@ # queue run cost in September 2026, and the most common reason a queue run # went red on a good tree (issue #5870). A regression there is real but rare, # and a day's delay in seeing it costs less than running the suites on every -# merge. The scheduled run diffs main against the previous tick and routes -# through FILTERS like any other event. A job is "queue" or "nightly", never -# both; check-ci-config.py enforces that. +# merge. The scheduled run diffs main against the commit the last successful +# scheduled run tested and routes through FILTERS like any other event. A job +# is "queue" or "nightly", never both; check-ci-config.py enforces that. # # "push" is reserved for work that can only happen once a commit is on main. # Adding "push" back to a test job would make every merge run it twice, once diff --git a/dev/ci/nightly-base.py b/dev/ci/nightly-base.py index b51012fe5d..0752999f6c 100644 --- a/dev/ci/nightly-base.py +++ b/dev/ci/nightly-base.py @@ -20,10 +20,14 @@ # # The nightly diffs main against that commit, so every commit that landed # since the last green nightly is covered exactly once, and a red nightly -# keeps the regressing commits in scope until a green one supersedes it. The -# `changes` job in ci.yml falls back to a time-based base when this prints -# nothing: the first scheduled run, or an API error, which is reported as a -# warning rather than failing the run. +# keeps the regressing commits in scope until a green one supersedes it. When +# this prints nothing -- the first scheduled run, or an API error, which is +# reported as a warning rather than failing the run -- the `changes` job in +# ci.yml has no base it can trust and runs the whole nightly tier instead. +# There is deliberately no narrower fallback: a guessed base that starts after +# a commit no nightly has covered yet would skip that commit's suites, let the +# run go green, and then become tomorrow's base, dropping the coverage for +# good. # # Needs GITHUB_TOKEN with `actions: read`, plus the GITHUB_REPOSITORY and # GITHUB_API_URL variables every job has. @@ -53,7 +57,8 @@ def main(): except (urllib.error.URLError, OSError, KeyError, ValueError) as e: # stderr, so the caller's command substitution stays empty. print( - f"::warning::could not list previous scheduled runs ({e}); using the time-based base", + f"::warning::could not list previous scheduled runs ({e}); " + "running the whole nightly tier", file=sys.stderr, ) return 0 diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index 7892b18d61..20f2eedf80 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -224,6 +224,10 @@ profiles other than 4.1, the Spark SQL suites for Spark 3.5 and 4.0, and the Ice everything else against the same tree, so nothing in the queue tier is repeated. A day with no merges, or with only documentation changes, runs nothing. +If the previous run cannot be found — the first nightly, an unreachable Actions API, or a base that +is no longer on `main` — the run has nothing to diff against and runs the whole nightly tier +instead of guessing at a range. + A red nightly has no pull request to show up on, so the run opens an issue labelled `ci-nightly-failure` that links the run and lists the jobs that failed. If one of those issues is already open, the run comments on it instead, so a failure that persists across several nights