From 3173cd26fbaae668c8445307d56b94154351684e Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 15 Sep 2026 00:40:36 -0600 Subject: [PATCH 1/3] 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/3] 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/3] 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