diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 43b5be5a5c0..7d591f8a99d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -18,12 +18,22 @@ ruleset in `.asf.yaml`. That splits CI into two tiers: 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 Spark 4.1 - `sql_hive` shards, Spark 3.4/3.5/4.0 and Iceberg 1.8/1.9/1.10, evaluated + `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. Every queue-only job has a `run-*` label that opts a pull request into it early, listed in the diagram below. +`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 +`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 +to block a merge. Adding the label fires a `labeled` event, and those runs +publish the advisory `Required Checks (label run)` name rather than the +required one, so a red 3.4 there changes nothing. It is the next push with +the label still applied that runs 3.4 under `Required Checks`, and with the +queue run gone that push is the only thing that makes a 3.4 failure blocking. + Heavy jobs have no `push` tier. The queue already tested the exact tree that lands, so re-running them on push to main would double the cost of every merge. Two jobs are still on `push`: `docs`, because it deploys to `asf-site` @@ -61,12 +71,11 @@ and TPC-H/TPC-DS caches on `main` stale until the next unrelated change. pr_build_linux (+ push, for cache) docs pr_build_macos run-macos-tests spark_4_1 (catalyst + sql_core) pr_benchmark_check run-benchmark-check iceberg_1_11 spark_4_1 sql_hive run-spark-4.1-hive-tests - spark_3_4 run-spark-3.4-tests 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 + 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 | | | +-----------------------------------+-----------------------------------+ @@ -96,7 +105,7 @@ and TPC-H/TPC-DS caches on `main` stale until the next unrelated change. | `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` | merge group, **or** PR with `run-spark-3.4-tests` | Spark 3.4 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 | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd3a0aa99cc..061ba582b41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,9 +26,12 @@ # 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 Spark 4.1 -# sql_hive shards, Spark 3.4/3.5/4.0 and Iceberg 1.8/1.9/1.10, +# 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. # +# 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. +# # 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 # already tested the exact commit that lands, so re-running the pipeline on @@ -255,7 +258,19 @@ jobs: spark_3_4: name: Spark SQL Tests (Spark 3.4) needs: changes - # Queue-only by default; PRs need the `run-spark-3.4-tests` label. + # Spark 3.4 is deprecated, so this is the one test job the merge queue does + # not run: it needs the `run-spark-3.4-tests` label on a pull request, or a + # manual workflow_dispatch. + # + # It is still in `required_checks.needs`, but be precise about what that + # buys. Adding the label fires a `labeled` event, and on that event the + # aggregator deliberately publishes the advisory `Required Checks (label + # run)` name instead of the required one (see its comment below), so a red + # 3.4 there leaves the commit's existing verdict alone. 3.4 only counts + # toward the required `Required Checks` on a pull_request event that is not + # `labeled` -- a push with the label already applied. With no queue run left + # to catch it, that push is the only thing that turns a 3.4 failure into a + # merge blocker. if: needs.changes.outputs.spark_3_4 == 'true' uses: ./.github/workflows/spark_sql_test_reusable.yml with: diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 6ccd22979ae..989dbc820b1 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -116,17 +116,23 @@ # 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", "spark_4_1", "iceberg_1_11"} -SPARK_OPT_IN = {"spark_3_4", "spark_3_5", "spark_4_0", "spark_4_1_hive"} +SPARK_OPT_IN = {"spark_3_5", "spark_4_0", "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"} BUILD_OPT_IN = {"build_macos", "benchmark"} QUEUE_TIER = PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN -ALL_JOBS = QUEUE_TIER | {"docs"} +ALL_JOBS = QUEUE_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. + # 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. ({"name": "merge_group"}, QUEUE_TIER), # Push to main is the site deploy plus the Linux build, which is there to # refresh main's actions/cache entries (see POLICY). Any other test job @@ -175,11 +181,23 @@ }, {"spark_4_1_hive"}, ), - # An opt-in label present on a pushed commit adds just that suite. + # 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 + # what keep it reachable at all. ( {"name": "pull_request", "action": "synchronize", "labels": ["run-spark-3.4-tests"]}, PR_TIER | {"spark_3_4"}, ), + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-spark-3.4-tests", + "labels": ["run-spark-3.4-tests"], + }, + {"spark_3_4"}, + ), ( {"name": "pull_request", "action": "synchronize", "labels": ["run-iceberg-tests"]}, PR_TIER | ICEBERG_OPT_IN, diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index 3380a1e4436..cf617d2e260 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -356,7 +356,11 @@ # docs deploys to asf-site, so it must not run from a pull request or from # the queue's throwaway branch. "docs": ["push"], - "spark_3_4": ["queue", "label:run-spark-3.4-tests"], + # Spark 3.4 is deprecated, so it is the one test job outside the queue + # tier: a failure there no longer blocks a merge. It stays runnable on + # 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, so it is the cheapest early diff --git a/docs/source/about/versioning_policy.md b/docs/source/about/versioning_policy.md index 1366def2b27..e5cb832f446 100644 --- a/docs/source/about/versioning_policy.md +++ b/docs/source/about/versioning_policy.md @@ -298,14 +298,17 @@ Spark project. Once upstream ends maintenance for a Spark minor, Comet removes i 1. **Deprecation.** The next Comet minor release after upstream maintenance ends marks the Spark minor as deprecated in the release notes and on the [Spark Version Compatibility](../user-guide/latest/compatibility/spark-versions.md) page. - Comet continues to build and publish binaries for the deprecated Spark minor during this - release cycle. -2. **Removal.** The following Comet minor release removes the Spark minor and stops publishing - binaries for it. - -This gives users at least one Comet minor release of prior notice before a Spark minor is -dropped. For example, Spark 3.4 was deprecated in Comet 1.0.0 and will be removed in Comet -1.1.0. + Comet continues to build and publish binaries for the deprecated Spark minor, but stops + gating merges on Apache Spark's own SQL test suite for that version, so a regression specific + to it is more likely to reach a release. +2. **Removal.** A later Comet minor release removes the Spark minor and stops publishing + binaries for it. Removal is not scheduled in advance; it happens once the remaining usage no + longer justifies the maintenance cost. + +This gives users at least one full Comet minor release of prior notice before a Spark minor is +dropped, and in practice usually more. For example, Spark 3.4 was deprecated in Comet 1.0.0. +Deprecation is the signal to plan a Spark upgrade; a deprecated version may be removed in any +subsequent minor release without further warning. ### Patch Versions diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index e6adb1c746d..c7b6c87de6b 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -21,7 +21,7 @@ under the License. Comet runs CI through GitHub Actions, and merges to `main` go through GitHub's merge queue. This page is the contributor's view of both: what runs on a pull request, what runs later in the queue, -how to opt a pull request into a queue-only suite, and what to do when a run fails. The +how to opt a pull request into a suite the PR tier skips, and what to do when a run fails. The mechanics behind the configuration are documented in [.github/workflows/README.md](https://github.com/apache/datafusion-comet/blob/main/.github/workflows/README.md). @@ -40,8 +40,9 @@ Which jobs run also depends on the event: | Iceberg Spark SQL tests, Iceberg 1.11 | yes | yes | | macOS build and Comet test suites | with label | yes | | Benchmark compile and lint check | with label | yes | -| Spark SQL tests, Spark 3.4 / 3.5 / 4.0 | 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 | 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 @@ -51,6 +52,16 @@ is the Linux build, which also runs on push so that the dependency caches on `ma pull request can only restore caches saved on its own branch or on `main`, and the queue's temporary branch takes its caches with it when it is deleted. +Spark 3.4 is the one suite in neither tier. [Spark 3.4 support is deprecated](../user-guide/latest/compatibility/spark-versions.md#spark-34), +so its Spark SQL suite no longer gates a merge. It remains available on demand: apply the +`run-spark-3.4-tests` label to run it against a pull request, or trigger `ci.yml` from the Actions +page with **Run workflow**, which runs every suite regardless of tier. A contributor touching Spark +3.4 code can still get a verdict before merging, but applying the label is only half of that: the +run it starts is advisory. Label the pull request and then push, and Spark 3.4 joins the required +verdict like any other suite. See +[Opting a pull request into a suite the PR tier skips](#opting-a-pull-request-into-a-suite-the-pr-tier-skips) +for the mechanics. + Every job's result feeds one flat job named `Required Checks`, and that is the only status check `main` requires. A job that is skipped because the change did not touch its inputs counts as a pass. A job that fails or is cancelled turns `Required Checks` red. @@ -59,9 +70,9 @@ Which tier a job belongs to is the `POLICY` table in `dev/ci/compute-changes.py` filters are the `FILTERS` table in the same file, and `dev/ci/check-ci-config.py` holds the test cases that pin both down. -## Opting a pull request into a queue-only suite +## Opting a pull request into a suite the PR tier skips -Each queue-only suite has a label that runs it on a pull request: +Each suite outside the PR tier has a label that runs it on a pull request: | Label | Runs | | -------------------------- | ---------------------------------------------------- | @@ -73,6 +84,10 @@ Each queue-only suite 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 Iceberg 1.8/1.9/1.10 | +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. + Apply a label from the pull request sidebar, or from the command line: ```sh @@ -84,6 +99,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. + 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, re-run the failed jobs from the Actions page, or remove and re-apply the label. @@ -102,6 +125,11 @@ does not cover. Some examples: - a change to the benchmark sources under `spark/src/test/scala/org/apache/spark/sql/benchmark` or to `native/*/benches` +Reach for `run-spark-3.4-tests` deliberately, since nothing else will run it: a change to +`spark/src/main/spark-3.4/`, to `dev/diffs/3.4.3.diff`, or to shared code whose Spark 3.4 behavior +you are unsure of. Without the label, a Spark 3.4 regression will not be caught before the change +lands. + Labels that gate nothing, such as `dependencies` or the type labels, also start a run. That run executes nothing and finishes in a minute. It is a consequence of GitHub firing the `labeled` event for every label; see the workflows README for why it is handled this way. @@ -163,11 +191,11 @@ re-armed automatically. ## Reproducing a suite failure locally -The queue-only Spark SQL suites run Spark's own test suite against Comet, with the version's diff -from `dev/diffs/` applied. See [Spark SQL Tests](spark-sql-tests.md) for how to run one locally, -and [Iceberg Spark Tests](iceberg-spark-tests.md) for the Iceberg equivalents. For the Comet test -suites that run on macOS, `make test-jvm` on a Mac runs the same suites the workflow does; the -macOS job differs from Linux only in the platform. +The Spark SQL suites outside the PR tier run Spark's own test suite against Comet, with the +version's diff from `dev/diffs/` applied. See [Spark SQL Tests](spark-sql-tests.md) for how to run +one locally, and [Iceberg Spark Tests](iceberg-spark-tests.md) for the Iceberg equivalents. For the +Comet test suites that run on macOS, `make test-jvm` on a Mac runs the same suites the workflow +does; the macOS job differs from Linux only in the platform. ## Changing CI itself diff --git a/docs/source/contributor-guide/spark-sql-tests.md b/docs/source/contributor-guide/spark-sql-tests.md index c90aca97f19..3b83c227c7c 100644 --- a/docs/source/contributor-guide/spark-sql-tests.md +++ b/docs/source/contributor-guide/spark-sql-tests.md @@ -168,9 +168,28 @@ 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 create a PR against Comet and let CI run the tests. When working with a -new Spark version, the `spark_sql_test.yaml` and `spark_sql_test_ansi.yaml` files will need updating with the -new version. +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. + +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. +Apply the label if your change touches `spark/src/main/spark-3.4/`, `dev/diffs/3.4.3.diff`, or +shared code whose Spark 3.4 behavior you are unsure of: + +```shell +gh pr edit --add-label run-spark-3.4-tests +``` + +The run that command starts is advisory: it reports under `Required Checks (label run)`, not the +`Required Checks` status `main` requires, so a Spark 3.4 failure there does not block the merge on +its own. The label stays applied, so the next push you make runs Spark 3.4 as part of the required +verdict. Read the label run's result before merging — with Spark 3.4 out of the merge queue, +nothing else will. + +See [Continuous Integration](ci.md) for how the tiers and labels work. When bringing up a new Spark +version, the version needs its own job in `.github/workflows/ci.yml` plus entries in `FILTERS` and +`POLICY` in `dev/ci/compute-changes.py`. ## Adding support for a new Spark major or minor version diff --git a/docs/source/user-guide/latest/compatibility/spark-versions.md b/docs/source/user-guide/latest/compatibility/spark-versions.md index 090a1df85e8..0a1c1924651 100644 --- a/docs/source/user-guide/latest/compatibility/spark-versions.md +++ b/docs/source/user-guide/latest/compatibility/spark-versions.md @@ -31,7 +31,11 @@ of the versioning policy. Spark 3.4.3 is supported with Java 11/17 and Scala 2.12/2.13. ```{warning} -Spark 3.4 support is deprecated as of the 1.0.0 release and will be removed in the 1.1.0 release. +Spark 3.4 support is deprecated as of the 1.0.0 release and will be removed in a future release. +Comet continues to build and publish Spark 3.4 binaries in the meantime, but Apache Spark's own +SQL test suite is no longer run against Spark 3.4 on every change: it runs only when a contributor +opts a pull request into it. Regressions specific to Spark 3.4 are therefore more likely to reach a +release than on the other supported versions. We recommend moving to Spark 3.5 or later. ``` ### Known Limitations diff --git a/docs/source/user-guide/latest/installation.md b/docs/source/user-guide/latest/installation.md index 7fc4a81d897..fbfddb6a580 100644 --- a/docs/source/user-guide/latest/installation.md +++ b/docs/source/user-guide/latest/installation.md @@ -46,19 +46,27 @@ We recommend only using Comet with Spark versions where we currently have both C Other versions may work well enough for development and evaluation purposes. ```{warning} -JDK 11 and Spark 3.4 support are deprecated as of the 1.0.0 release and will be removed in the 1.1.0 release. -We recommend moving to JDK 17 or later and Spark 3.5 or later. +JDK 11 support is deprecated as of the 1.0.0 release and will be removed in the 1.1.0 release. We +recommend moving to JDK 17 or later. + +Spark 3.4 support is deprecated as of the 1.0.0 release and will be removed in a future release. +Apache Spark's own SQL test suite is no longer run against Spark 3.4 on every change; it runs only +on demand. We recommend moving to Spark 3.5 or later. ``` | Spark Version | Java Version | Scala Version | Comet Tests in CI | Spark SQL Tests in CI | | ------------- | ------------ | ------------- | ----------------- | --------------------- | -| 3.4.3 | 11/17 | 2.12/2.13 | Yes | Yes | +| 3.4.3 | 11/17 | 2.12/2.13 | Yes | On demand | | 3.5.9 | 11/17 | 2.12/2.13 | Yes | Yes | | 4.0.4 | 17/21 | 2.13 | Yes | Yes | | 4.1.3 | 17/21 | 2.13 | Yes | Yes | Note that we do not test the full matrix of supported Java and Scala versions in CI for every Spark version. +"On demand" in the table above means the suite is not run automatically before a change is merged. +A contributor can still run it against an individual pull request, but Spark 3.4 is no longer +covered by default. + Experimental support is provided for the following versions of Apache Spark and is intended for development/testing use only and should not be used in production yet.