You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by ag...@apache.org on 2026/09/11 21:07:42 UTC
(datafusion-comet) branch main updated: ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue (#5843)
This is an automated email from the ASF dual-hosted git repository.
andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new 2357fcd7ba ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue (#5843)
2357fcd7ba is described below
commit 2357fcd7ba47bad71e4d2adb0d21ae04e6a02201
Author: Andy Grove <ag...@apache.org>
AuthorDate: Fri Sep 11 15:07:36 2026 -0600
ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue (#5843)
* ci: move the job routing policy out of ci.yml expressions and into compute-changes.py
Every heavy job in `ci.yml` carried a four-line `${{ }}` expression combining
a path-filter output, an event-name test, an opt-in label test, and a special
case for `labeled` events. Ten jobs, ten near-identical copies, none of them
testable outside a real workflow run.
Fold that policy into `dev/ci/compute-changes.py` next to the path filters it
was already being ANDed with. Each job's gate becomes:
if: needs.changes.outputs.spark_3_5 == 'true'
and the routing lives in one readable table:
POLICY = {
"spark_3_5": ["pr", "push"],
"spark_3_4": ["push", "label:run-spark-3.4-tests"],
...
}
`ci.yml` loses 62 lines net. No behaviour changes: the `changes` job now
receives the event name, action, added label and PR labels, and applies the
same rules the expressions did.
The point of moving it is that it can now be tested. `check-ci-config.py`
grows `POLICY_CASES`, which pins the expected job set for each event shape --
including that a non-gating label such as dependabot's `dependencies` starts
nothing, which is issue #5007 and was previously only enforceable by reading
YAML carefully.
Writing `"pr"` alongside a `"label:"` tier reads as "runs on every PR, and also
when labelled" but the label check wins and the "pr" is dead, so
`check-ci-config.py` now rejects that combination rather than letting it be a
silent no-op.
Verified equivalent with a differential harness: the pre-refactor `if:`
expressions were extracted from ci.yml at the merge base, translated to Python,
and evaluated against the new POLICY over 1464 (job, event) combinations --
every event name, pull_request action, label set and added-label pairing. Zero
mismatches, and three seeded regressions (renaming a gating label, dropping a
label gate, dropping the labeled-event narrowing) are each caught.
Groundwork for #5838: adding the merge queue then means adding one tier to
POLICY rather than editing ten YAML expressions.
* ci: add a Required Checks aggregator job so main can have a required status check
`main` currently requires no status checks, only one approving review. Adding
one is blocked by how the umbrella workflow reports: every heavy job in
`ci.yml` is a thin caller of a reusable workflow, and the check name a caller
publishes depends on whether it ran.
skipped by `if:` one check run named exactly `PR Build (Linux)`
actually ran only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]` and
friends, and no bare `PR Build (Linux)` at all
Confirmed against bdd2aeb (md-only, everything skipped) and 36caf8e (heavy
jobs ran). No name is reported in both cases, so requiring the bare name would
block every code change and requiring a nested name would block every docs-only
change. Both hang waiting for a check that never arrives rather than failing,
and a required context that never reports also blocks the merge that would fix
`.asf.yaml` -- only INFRA can remove a required check by hand at that point.
Add `required_checks`, a flat job that `needs:` every other job and reports on
every event. It runs `if: always()` and treats `skipped` as a pass, so it goes
red only when an upstream job reports `failure` or `cancelled`.
Nothing requires it yet. This lands on its own so the check can be observed on
real pull requests before `.asf.yaml` names it, which is the step that is
expensive to get wrong.
`dev/ci/check-ci-config.py` gains two invariants: every `ci.yml` job except
`docs` must appear in `required_checks.needs`, and once `.asf.yaml` does
declare a required context, the job's `name:` must match it.
Part of #5838.
* ci: give the Required Checks job an empty permissions block
The aggregator reads only the `needs` context, so it has no use for the
GITHUB_TOKEN. Declaring `permissions: {}` on the job drops the token
entirely and clears the CodeQL "Workflow does not contain permissions"
finding, without touching the token scope any other job in ci.yml gets.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: tighten the required-checks invariants in check-ci-config.py
Widen the job-id regexes to accept `-` and uppercase, which GitHub allows.
A `spark-4-2:` job missing from `required_checks.needs` passed the coverage
check silently before this change.
Scope the `.asf.yaml` parse to the `main:` entry under `protected_branches`.
It previously collected every `contexts:` list in the file, so a release
branch requiring its own context produced a false failure that blamed main.
Drop the em dashes from the new README prose.
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: retry or tolerate infra-only step failures on the required path
Once Required Checks is a required context, any red job evicts the PR from
the merge queue. The first two runs on this branch each failed in a step
that runs after the test verdict is known or before any test has started,
and in neither case was a test involved.
- java-test: mark the test-report upload continue-on-error. It runs on green
jobs, nothing downstream consumes the reports, and a FinalizeArtifact 403
must not turn a passing run red. The failure-only uploads are unchanged.
- iceberg_spark_test_reusable: route the shard-inventory upload through
upload-artifact-retry. iceberg-spark-shard-coverage consumes it, so it
already fell under the README rule for artifacts a later job downloads.
- pr_build_linux: split Lint Scala (syntactic) into a retried no-op
`cs launch scalafix -- --version` that populates the cache, then the real
check under `--mode offline`, so a nonzero exit there is only ever a lint
violation.
- ci.yml preflight: retry the actionlint download and fetch the installer
to a file instead of piping it into bash.
Closes #5860
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: move the Spark 3.4/3.5/4.0 suites behind a merge queue
Turns on GitHub's merge queue for `main` and splits CI into two tiers, so an
iterating pull request stops paying for suites that only need to be green at
merge time.
PR tier build Linux/macOS, benchmark, Spark 4.1, Iceberg 1.11
queue tier all of the above, plus Spark 3.4/3.5/4.0 and Iceberg 1.8/1.9/1.10
push main docs deploy only
Because the routing policy now lives in `POLICY` in compute-changes.py, the
retiering is a table edit rather than ten rewritten `${{ }}` expressions:
`"push"` becomes `"queue"`, and Spark 3.5 gains a `run-spark-3.5-tests` label
so it can still be opted into from a pull request. `ci.yml` only gains the
`merge_group` trigger and the base/head SHAs to diff a queue branch against.
Spark 3.5 is the one that changes for contributors: it ran on every pull
request and is now queue-only. Spark 4.1 stays in the PR tier because it is
the default build profile and so the cheapest early warning that a change is
wrong.
Dropping the `push` tier from the heavy jobs is load-bearing, not cleanup. The
queue tests the exact tree that lands, so keeping the old push-to-main runs
would make every merge pay for CI twice and total usage would go up rather
than down. Iceberg 1.8/1.9/1.10 move into the queue for the same reason: they
were push-only, so they had nowhere else to run.
The queue itself is configured in `.asf.yaml`. Rulesets there accept a raw
Rulesets API payload, so a `merge_queue` rule needs no INFRA ticket; this is
the same approach apache/iceberg-python and apache/seatunnel use. GitHub
evaluates the ruleset and classic branch protection together, so the existing
one-approving-review requirement is unaffected, and release branches keep plain
protection because merge queue rules reject wildcard ref patterns.
`Required Checks`, added in the previous commit, becomes the single required
status check and therefore the only thing the queue waits on. `POLICY_CASES`
in check-ci-config.py is updated to pin the new tiers, including that push to
main now runs the site deploy and nothing else.
Two follow-ups this does not cover: `run-spark-3.4-tests` and
`run-spark-3.5-tests` still have to be created in repository settings, and a
flaky test in the queue tier now blocks everyone's merges rather than one pull
request, which is worth a heads-up on dev@ before this is enabled.
Part of #5838.
* ci: move the macOS build and benchmark check behind the merge queue
The Linux build already covers rustfmt and the Rust/JVM compile on every
pull request, and macOS runners are the scarcest capacity we have. The
benchmark check only compiles and lints benchmark sources, so a break
there cannot change a PR's verdict. Both now run in the queue tier, with
`run-macos-tests` and `run-benchmark-check` labels as escape hatches.
Also update the per-job comments in ci.yml that still described the
opt-in suites as main-only, and drop a duplicated `required_checks` box
from the pipeline diagram in the workflows README.
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: retry artifact downloads the way uploads already are
A ListArtifacts request answered "(403) Forbidden: Error from
intermediary" fails actions/download-artifact before a byte is fetched.
It is usually the first step of a test shard, so the shard never runs
and the job is red for a reason that has nothing to do with the change.
Once Required Checks gates the merge queue, that evicts the PR.
Add download-artifact-retry alongside upload-artifact-retry: same three
attempts and backoff, the subset of inputs this repository uses, and
nothing to undo between attempts. Use it at every download that has a
checkout; merge-fallback-logs deliberately skips checkout, so it stays
on the plain action. check-ci-config.py recognises both spellings when
pairing downloads with their producers.
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: keep the Linux build on push to main so its caches stay fresh
A pull request can restore actions/cache entries saved on its own branch
or on main, and nowhere else. The merge queue runs on a throwaway
gh-readonly-queue/* branch, so the caches it saves are deleted with it.
With no test job left on push, a Cargo.lock or pom.xml change would
leave main's cargo-registry, Maven and TPC-H/TPC-DS caches stale, and
every later pull request would pay the delta on top of the restore-keys
prefix match.
Put build_linux back on push, as the one job that refreshes them, and
say why in POLICY, the policy test case and the workflows README.
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: route download-artifact-retry to the jobs that consume it
FILTERS listed the upload wrapper but not the download one, so an edit
confined to .github/actions/download-artifact-retry/ set every output
false and none of its ten Linux/macOS/Spark/Iceberg consumers ran, on a
pull request or in the queue alike.
Add the path to the same ten filters, and a ROUTING_CASES entry so a
future deletion is caught the way the upload wrapper's already is.
* ci: keep the checkout-free fallback-log job on the published download action
merge-fallback-logs runs on its own runner with no checkout, so it cannot
load a local composite action. Its own comment already said so, but the
`uses:` had been switched to ./.github/actions/download-artifact-retry,
which means the merged fallback-log artifact would simply not be produced.
No current caller sets collect-fallback-logs, so nothing exercises it.
Restore actions/download-artifact@v8 there, and add a fifth invariant to
check-ci-config.py: any `uses: ./.github/actions/...` in a workflow needs
an actions/checkout earlier in the same job. Verified by reintroducing the
bad `uses:` and watching the check fail on exactly that step.
* docs: correct what the merge queue's entry limits actually do
The description of max_entries_to_merge was wrong. GitHub documents that
merge limits do not combine merge_group builds: every queue entry gets its
own build of itself plus everything ahead of it, and the limit only caps
how many already-green entries land in one merge operation. So it saves no
CI, and "one pipeline validates up to five PRs" was not a real property.
max_entries_to_build is the dial that matters. It caps how many pipelines
are in flight, and with them how fast the queue drains: about 19 merges a
day at 2 and the ~2.5h pipeline we see today, against a median of 6 merges
a day and a busiest day of 22 over the last month. Left at 2 for now, but
worth raising if the queue backs up.
The ALLGREEN comment was also describing HEADGREEN's behaviour. Under
ALLGREEN every entry's own build must be green, not just the head's.
None of this changes the case for the split: the saving comes from the PR
tier being small, not from batching inside the queue.
Thanks to @sunchao for catching the claim.
---------
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
Co-authored-by: test <a...@b.c>
---
.asf.yaml | 68 ++++++++
.../actions/download-artifact-retry/action.yaml | 107 ++++++++++++
.github/workflows/README.md | 182 +++++++++++++++------
.github/workflows/ci.yml | 48 ++++--
.github/workflows/iceberg_spark_test_reusable.yml | 8 +-
.github/workflows/pr_build_linux.yml | 6 +-
.github/workflows/pr_build_macos.yml | 2 +-
.github/workflows/spark_sql_test_reusable.yml | 6 +-
dev/ci/check-ci-config.py | 105 ++++++++++--
dev/ci/compute-changes.py | 59 +++++--
.../contributor-guide/iceberg-spark-tests.md | 8 +-
11 files changed, 501 insertions(+), 98 deletions(-)
diff --git a/.asf.yaml b/.asf.yaml
index 21562b811c..b99b3a65f3 100644
--- a/.asf.yaml
+++ b/.asf.yaml
@@ -46,6 +46,19 @@ github:
main:
required_pull_request_reviews:
required_approving_review_count: 1
+ required_status_checks:
+ # `Required Checks` is the aggregator job at the bottom of
+ # .github/workflows/ci.yml. It is deliberately the only entry: the
+ # individual heavy jobs publish a different check name depending on
+ # whether they ran or were skipped, so none of them can be required
+ # directly. dev/ci/check-ci-config.py keeps this string in sync with
+ # the job's `name:`.
+ #
+ # Careful when editing this list. A context that never reports blocks
+ # every merge to main, including the merge that would fix this file,
+ # and recovering from that needs an INFRA Jira ticket.
+ contexts:
+ - "Required Checks"
branch-0.1:
required_pull_request_reviews:
required_approving_review_count: 1
@@ -97,8 +110,63 @@ github:
branch-0.17:
required_pull_request_reviews:
required_approving_review_count: 1
+ # GitHub's merge queue. Classic `protected_branches` above has no way to
+ # express this, but .asf.yaml rulesets accept a raw Rulesets API payload, so
+ # no INFRA ticket is needed. Same approach as apache/iceberg-python and
+ # apache/seatunnel; GitHub evaluates rulesets and classic branch protection
+ # together and applies the most restrictive result.
+ #
+ # Only the default branch is queued. Merge queue rules do not support
+ # wildcard ref patterns, so the branch-N.M release branches keep plain
+ # branch protection.
+ rulesets:
+ - name: Merge Queue
+ target: branch
+ enforcement: active
+ bypass_actors:
+ # apache/root - ASF Infra, so a wedged queue can always be recovered.
+ - actor_id: 118420
+ actor_type: Team
+ bypass_mode: always
+ conditions:
+ ref_name:
+ include:
+ - "~DEFAULT_BRANCH"
+ exclude: []
+ rules:
+ - type: merge_queue
+ parameters:
+ # Matches enabled_merge_buttons above: squash is the only method
+ # the repository allows.
+ merge_method: SQUASH
+ # ALLGREEN: every entry's own build has to be green, not just the
+ # last one's. HEADGREEN would let an entry whose build failed ride
+ # in on a green head, which papers over a flake but equally papers
+ # over a real failure that a later entry happens to mask.
+ grouping_strategy: ALLGREEN
+ # Speculation depth: how many merge_group builds run at once. Each
+ # queue entry gets its own build of itself plus everything ahead of
+ # it, and merge limits do not combine those builds, so this is the
+ # throughput dial. At 1 every merge serializes behind a ~2.5h
+ # pipeline; 2 sustains roughly 19 merges a day, against a median of
+ # 6 and a busiest-day 22 over the last month. Raise it if the queue
+ # backs up, at the price of that many concurrent pipelines.
+ max_entries_to_build: 2
+ # These only cap how many already-green entries land in one merge;
+ # they do not batch the builds. No minimum and no wait, so a lone
+ # PR merges the moment its build is green.
+ min_entries_to_merge: 1
+ min_entries_to_merge_wait_minutes: 0
+ max_entries_to_merge: 5
+ # Full pipeline runs have been observed at 2h04m-2h25m, on top of
+ # ASF runner scheduling delays. Anything under ~4h would evict
+ # healthy entries.
+ check_response_timeout_minutes: 300
pull_requests:
allow_update_branch: true
+ # Lets a committer queue a PR before its checks have finished, which is
+ # most of the point of having a queue.
+ allow_auto_merge: true
# publishes the content of the `asf-site` branch to
# https://datafusion.apache.org/comet/
publish:
diff --git a/.github/actions/download-artifact-retry/action.yaml b/.github/actions/download-artifact-retry/action.yaml
new file mode 100644
index 0000000000..28e0e4464e
--- /dev/null
+++ b/.github/actions/download-artifact-retry/action.yaml
@@ -0,0 +1,107 @@
+# 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.
+
+name: "Download Artifact (with retry)"
+description: >
+ Drop-in replacement for actions/download-artifact that retries the download
+ three times. The artifact client only retries 429/500/502/503/504, so a
+ ListArtifacts answered "(403) Forbidden: Error from intermediary" fails the
+ step before a single byte is fetched. There is no input to widen that list,
+ Actions has no built-in step retry, and third-party retry wrappers are not on
+ the Apache allowed-actions list. See .github/workflows/README.md.
+
+ Unlike the upload wrapper, a retry here has no side effect to undo: a failed
+ attempt leaves at most a partial extraction under `path`, which the next
+ attempt overwrites file by file.
+
+# Inputs are the subset of actions/download-artifact@v8 that this repository
+# uses. The boolean default has to be a real 'false' string because
+# core.getBooleanInput() throws on an empty value.
+inputs:
+ name:
+ description: 'Name of the artifact to download (empty means every artifact in the run)'
+ required: false
+ default: ''
+ path:
+ description: 'Destination path (empty means the workspace)'
+ required: false
+ default: ''
+ pattern:
+ description: 'Glob matching the artifacts to download; ignored when name is set'
+ required: false
+ default: ''
+ merge-multiple:
+ description: 'Extract every matched artifact straight into path rather than into one directory each'
+ required: false
+ default: 'false'
+
+# `||` yields the first non-empty operand, so this picks whichever attempt ran.
+outputs:
+ download-path:
+ description: 'Path of artifact download'
+ value: ${{ steps.attempt-1.outputs.download-path || steps.attempt-2.outputs.download-path || steps.attempt-3.outputs.download-path }}
+
+runs:
+ using: "composite"
+ steps:
+ # continue-on-error keeps a failed attempt from failing the job while still
+ # recording outcome == 'failure', which the later attempts gate on. The last
+ # attempt omits it so a genuinely missing artifact still fails loudly.
+ - name: Download ${{ inputs.name || inputs.pattern || 'all artifacts' }} (attempt 1 of 3)
+ id: attempt-1
+ uses: actions/download-artifact@v8
+ continue-on-error: true
+ with:
+ name: ${{ inputs.name }}
+ path: ${{ inputs.path }}
+ pattern: ${{ inputs.pattern }}
+ merge-multiple: ${{ inputs.merge-multiple }}
+
+ - name: Wait before retrying ${{ inputs.name || inputs.pattern || 'all artifacts' }}
+ if: ${{ steps.attempt-1.outcome == 'failure' }}
+ shell: bash
+ run: |
+ echo "::warning::Download of '${{ inputs.name || inputs.pattern || 'all artifacts' }}' failed; retrying in 15s (attempt 2 of 3)."
+ sleep 15
+
+ - name: Download ${{ inputs.name || inputs.pattern || 'all artifacts' }} (attempt 2 of 3)
+ id: attempt-2
+ if: ${{ steps.attempt-1.outcome == 'failure' }}
+ uses: actions/download-artifact@v8
+ continue-on-error: true
+ with:
+ name: ${{ inputs.name }}
+ path: ${{ inputs.path }}
+ pattern: ${{ inputs.pattern }}
+ merge-multiple: ${{ inputs.merge-multiple }}
+
+ - name: Wait before final retry of ${{ inputs.name || inputs.pattern || 'all artifacts' }}
+ if: ${{ steps.attempt-1.outcome == 'failure' && steps.attempt-2.outcome == 'failure' }}
+ shell: bash
+ run: |
+ echo "::warning::Download of '${{ inputs.name || inputs.pattern || 'all artifacts' }}' failed again; retrying in 45s (attempt 3 of 3)."
+ sleep 45
+
+ - name: Download ${{ inputs.name || inputs.pattern || 'all artifacts' }} (attempt 3 of 3)
+ id: attempt-3
+ if: ${{ steps.attempt-1.outcome == 'failure' && steps.attempt-2.outcome == 'failure' }}
+ uses: actions/download-artifact@v8
+ with:
+ name: ${{ inputs.name }}
+ path: ${{ inputs.path }}
+ pattern: ${{ inputs.pattern }}
+ merge-multiple: ${{ inputs.merge-multiple }}
diff --git a/.github/workflows/README.md b/.github/workflows/README.md
index f98918b24a..34765f74a3 100644
--- a/.github/workflows/README.md
+++ b/.github/workflows/README.md
@@ -6,13 +6,36 @@ workflows. This README is ignored by the runner.
## Pipeline overview
A single umbrella workflow (`ci.yml`) orchestrates everything that runs on
-pull requests and pushes to `main`. The umbrella runs cheap **preflight**
+pull requests and in the merge queue. The umbrella runs cheap **preflight**
checks first, computes which heavy jobs are relevant to the change, and only
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:
+
+- **PR tier** (`pr`): fast feedback while a change is being iterated on.
+ The Linux build, Spark 4.1 and Iceberg 1.11.
+- **Queue tier** (`queue`): the authoritative gate. Everything the PR tier
+ runs, plus the macOS build, the benchmark compile check, Spark 3.4/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.
+
+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`
+and has to run after the commit is on main, and `pr_build_linux`, because of
+`actions/cache` scoping. A pull request can only restore caches saved on its
+own branch or on `main`, and the queue runs on a throwaway
+`gh-readonly-queue/*` branch whose caches are deleted with it. Without a push
+run, a `Cargo.lock` or `pom.xml` change would leave the cargo-registry, Maven
+and TPC-H/TPC-DS caches on `main` stale until the next unrelated change.
+
```
- pull_request | push to main | workflow_dispatch
+ pull_request | merge_group | push to main | workflow_dispatch
|
v
+-----------------------+
@@ -33,14 +56,16 @@ is a `workflow_call` reusable invoked from the umbrella.
+-----------------------------------+-----------------------------------+
| | |
v v v
- every PR + push push to main only PR with label, or push
- --------------- ----------------- ----------------------
- pr_build_linux docs spark_3_4 run-spark-3.4-tests
- pr_build_macos spark_4_0 run-spark-4.0-tests
- pr_benchmark_check iceberg_1_8 run-iceberg-tests
- spark_3_5 iceberg_1_9 run-iceberg-tests
- spark_4_1 iceberg_1_10 run-iceberg-tests
- iceberg_1_11
+ PR + queue tier push to main only queue tier, or PR with label
+ --------------- ----------------- ---------------------------
+ pr_build_linux (+ push, for cache) docs pr_build_macos run-macos-tests
+ spark_4_1 pr_benchmark_check run-benchmark-check
+ iceberg_1_11 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
| | |
+-----------------------------------+-----------------------------------+
@@ -60,23 +85,23 @@ is a `workflow_call` reusable invoked from the umbrella.
## What runs when
-| Job in `ci.yml` | Triggered by | Routing rule |
-| -------------------- | --------------------------------------------------- | ----------------------------------- |
-| `preflight` | every PR / push to main / dispatch / PR label added | none (always runs) |
-| `changes` | every PR / push to main / dispatch / PR label added | runs `dev/ci/compute-changes.py` |
-| `pr_build_linux` | PR or push, paths matched | `dev/ci/compute-changes.py` |
-| `pr_build_macos` | PR or push, paths matched | `dev/ci/compute-changes.py` |
-| `pr_benchmark_check` | PR or push, paths matched | benchmark sources only |
-| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` |
-| `spark_3_5` | PR or push, paths matched | Spark 3.5 sources |
-| `spark_4_1` | PR or push, paths matched | Spark 4.1 sources |
-| `spark_3_4` | push, **or** PR with `run-spark-3.4-tests` label | Spark 3.4 sources |
-| `spark_4_0` | push, **or** PR with `run-spark-4.0-tests` label | Spark 4.0 sources |
-| `iceberg_1_11` | PR or push, paths matched | Iceberg sources |
-| `iceberg_1_8` | push, **or** PR with `run-iceberg-tests` label | Iceberg sources |
-| `iceberg_1_9` | push, **or** PR with `run-iceberg-tests` label | Iceberg sources |
-| `iceberg_1_10` | push, **or** PR with `run-iceberg-tests` label | 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 | `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 |
+| `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 | Spark 4.1 sources |
+| `spark_3_4` | merge group, **or** PR with `run-spark-3.4-tests` | 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) |
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
@@ -114,6 +139,10 @@ Two rules keep those runs from corrupting the PR's status:
`run-spark-4.1-tests` gates nothing: `spark_4_1` already runs on every PR.
+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.
+
## Standalone workflows (not under the umbrella)
These workflows have their own triggers because they fire on events the
@@ -155,9 +184,11 @@ 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,
- `"push"` for push to main, `"label:<name>"` for opt-in on a labelled pull
- request. `"pr"` and `"label:"` are mutually exclusive. `workflow_dispatch`
- always runs everything.
+ `"queue"` for the merge queue, `"push"` for push to main, `"label:<name>"`
+ 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`.
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
@@ -223,6 +254,18 @@ on jobs that are already red. Its test-report upload also runs on green jobs
and is `continue-on-error: true`: nothing downstream consumes the reports, and
a `FinalizeArtifact` 403 must not turn a passing test run into a red check.
+**Artifact download.** `actions/download-artifact` has the same narrow retry
+list, so a `ListArtifacts` answered `(403) Forbidden: Error from intermediary`
+fails the job before a byte is fetched, most often the first step of a test
+shard that then never runs. `./.github/actions/download-artifact-retry` wraps
+it the same way: three attempts, 15s then 45s backoff, same inputs and
+`download-path` output. A retry has nothing to undo, since a failed attempt
+leaves at most a partial extraction that the next one overwrites. The
+`merge-fallback-logs` job stays on the plain action because it skips checkout,
+which a local action needs; `dev/ci/check-ci-config.py` enforces that pairing
+for every `uses: ./.github/actions/...` in a workflow, and treats both
+spellings as a download when pairing consumers with producers.
+
**Tool downloads.** `Lint Scala (syntactic)` splits the coursier download from
the lint: a `Fetch scalafix` step retries a no-op `cs launch ... -- --version`
three times, and the check itself then runs `cs launch --mode offline` against
@@ -241,34 +284,73 @@ the whole pipeline by hand.
`./mvnw --version` with exponential backoff, so a failed download of the Maven
distribution does not surface as a test failure.
+## Merge queue
+
+`.asf.yaml` declares a `Merge Queue` ruleset for the default branch, so `main`
+is only writable through the queue. `.asf.yaml` rulesets accept a raw GitHub
+Rulesets API payload, which is how a `merge_queue` rule gets set without an
+INFRA ticket.
+
+When a PR is queued, GitHub builds a temporary `gh-readonly-queue/main/...`
+branch containing the PR's commits on top of the current `main` and of every
+entry ahead of it in the queue, then fires a `merge_group` event. `ci.yml` runs
+against that branch and the entry merges when `Required Checks` is green. That
+is what makes the queue tier meaningful: it tests the merge result, not the PR
+head, so a semantic conflict between two PRs that each pass in isolation is
+caught before either lands.
+
+The `merge_queue` rule parameters in `.asf.yaml` are the tuning dials, and
+`max_entries_to_build: 2` is the one that matters. Every entry gets its own
+`merge_group` build — [merge limits do not combine
+builds](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue) —
+so this caps how many pipelines are in flight, and with them how fast the queue
+drains: roughly 19 merges a day at the ~2.5h pipeline we see today.
+`max_entries_to_merge: 5` only says how many already-green entries land in one
+merge operation, and saves no CI at all. The saving in this design comes from
+the PR tier being small, not from batching inside the queue.
+`check_response_timeout_minutes: 300` has to stay comfortably above the slowest
+observed pipeline plus ASF runner scheduling delay, or healthy entries get
+evicted.
+
+A flaky test in the queue tier blocks everyone's merges, not just one PR. That
+raises the bar on flakiness relative to when these suites only ran post-merge.
+
## Branch protection
-`.asf.yaml` declares no `required_status_checks` for `main` today, only
-`required_approving_review_count: 1`.
+`main` is protected by two things that GitHub evaluates together, applying the
+most restrictive result:
-Adding one is not as simple as naming a job, because a caller of a reusable
-workflow publishes a _different check name_ depending on whether it ran:
+- classic branch protection, from `github.protected_branches.main` in
+ `.asf.yaml`: one approving review, and `Required Checks` as the sole
+ required status check;
+- the `Merge Queue` ruleset, from `github.rulesets` in the same file.
+
+Release branches (`branch-N.M`) keep plain branch protection with no queue.
+Merge queue rules do not accept wildcard ref patterns, so the ruleset targets
+`~DEFAULT_BRANCH` only.
+
+`apache/root` (ASF Infra, team id `118420`) is a bypass actor on the ruleset,
+so a wedged queue can always be recovered without a Jira ticket.
+
+`Required Checks` is the one context `main` requires, because a caller of a
+reusable workflow publishes a _different check name_ depending on whether it
+ran:
| Caller state | Check runs published |
| ---------------- | -------------------------------------------------------- |
| skipped by `if:` | one run named exactly `PR Build (Linux)`, `skipped` |
| ran | only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]`, etc. |
-No name is reported in both cases. Requiring the bare name would block every
-code change; requiring a nested name would block every docs-only change. Both
-hang waiting for a check that never arrives rather than failing, and a required
-context that never reports also blocks the merge that would fix `.asf.yaml`.
-Recovering from that needs an INFRA Jira ticket.
-
-The `required_checks` job at the bottom of `ci.yml` exists to be the one name
-that is safe to require. It is flat, so it reports on every event; it runs
-`if: always()` and treats `skipped` as a pass, so it only goes red when an
-upstream job reports `failure` or `cancelled`.
+No name is reported in both cases, so neither can be required directly.
+`required_checks` is flat, reports on every event, runs `if: always()` and
+treats `skipped` as a pass, so it only goes red on `failure` or `cancelled`.
+Editing `required_status_checks` deserves care. A context that never reports
+blocks every merge to `main`, including the merge that would revert the
+mistake, and only INFRA can remove a required check by hand at that point.
`dev/ci/check-ci-config.py` enforces that every `ci.yml` job except `docs`
-appears in `required_checks.needs`, and that the job's `name:` is the
-expression that routes `labeled` runs to a separate name (see "Label events"
-above). Once `.asf.yaml` does declare a required context for `main`, it also
-enforces that the commit-run name still matches it and that the label-run name
-is never the one required. All of that is silent when broken and expensive to
-recover from.
+appears in `required_checks.needs`, that the job's `name:` is the expression
+that routes `labeled` runs to a separate name (see "Label events" above), that
+the commit-run name matches the context `.asf.yaml` requires for `main`, and
+that the label-run name is never the one required. All of that is silent when
+broken. What it cannot catch is a job that is configured never to run.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 1bb96a43ab..9754b89d4d 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -17,7 +17,24 @@
# Top-level CI orchestrator: runs cheap preflight checks first, then fans out
# to the long-running test/build workflows only if preflight passed and the
-# PR/push touched files relevant to that workflow.
+# change touched files relevant to that workflow.
+#
+# 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 + Iceberg 1.11.
+# merge_group the authoritative gate. Everything the PR tier runs, plus
+# the macOS build, the benchmark compile check, Spark
+# 3.4/3.5/4.0 and Iceberg 1.8/1.9/1.10, evaluated against
+# the merge result rather than the PR head.
+#
+# 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
+# push to main would double the cost of every merge. Two exceptions: `docs`
+# deploys to asf-site and must run after the commit is on main, and the Linux
+# build runs on push so that main's actions/cache entries stay fresh (caches
+# saved on the queue's throwaway branch are deleted with it).
name: CI
@@ -33,6 +50,7 @@ concurrency:
on:
pull_request:
types: [opened, synchronize, reopened, labeled]
+ merge_group:
push:
branches:
- main
@@ -163,6 +181,8 @@ jobs:
PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }}
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+ MQ_BASE_SHA: ${{ github.event.merge_group.base_sha }}
+ MQ_HEAD_SHA: ${{ github.event.merge_group.head_sha }}
PUSH_BEFORE: ${{ github.event.before }}
PUSH_AFTER: ${{ github.sha }}
run: |
@@ -175,6 +195,10 @@ jobs:
:
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
+ # The queue branch is built on top of base_sha, so this covers every
+ # entry batched into this merge group, not just the newest one.
+ git diff --name-only "$MQ_BASE_SHA"..."$MQ_HEAD_SHA" > changed_files.txt
else
# push to main; first push to a branch has all-zero before sha
if [[ "$PUSH_BEFORE" =~ ^0+$ ]]; then
@@ -208,12 +232,14 @@ jobs:
pr_build_macos:
name: PR Build (macOS)
needs: changes
+ # Queue-only by default; PRs need the `run-macos-tests` label.
if: needs.changes.outputs.build_macos == 'true'
uses: ./.github/workflows/pr_build_macos.yml
pr_benchmark_check:
name: PR Benchmark Check
needs: changes
+ # Queue-only by default; PRs need the `run-benchmark-check` label.
if: needs.changes.outputs.benchmark == 'true'
uses: ./.github/workflows/pr_benchmark_check.yml
@@ -227,7 +253,7 @@ jobs:
spark_3_4:
name: Spark SQL Tests (Spark 3.4)
needs: changes
- # Main-only by default; PRs need the `run-spark-3.4-tests` label.
+ # Queue-only by default; PRs need the `run-spark-3.4-tests` label.
if: needs.changes.outputs.spark_3_4 == 'true'
uses: ./.github/workflows/spark_sql_test_reusable.yml
with:
@@ -238,6 +264,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.
if: needs.changes.outputs.spark_3_5 == 'true'
uses: ./.github/workflows/spark_sql_test_reusable.yml
with:
@@ -248,7 +275,7 @@ jobs:
spark_4_0:
name: Spark SQL Tests (Spark 4.0)
needs: changes
- # Main-only by default; PRs need the `run-spark-4.0-tests` label. Swapped
+ # 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.
if: needs.changes.outputs.spark_4_0 == 'true'
@@ -271,7 +298,7 @@ jobs:
iceberg_1_8:
name: Iceberg Spark SQL Tests (Iceberg 1.8)
needs: changes
- # Main-only by default; PRs need the `run-iceberg-tests` label.
+ # Queue-only 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:
@@ -284,7 +311,7 @@ jobs:
iceberg_1_9:
name: Iceberg Spark SQL Tests (Iceberg 1.9)
needs: changes
- # Main-only by default; PRs need the `run-iceberg-tests` label.
+ # Queue-only 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:
@@ -297,7 +324,7 @@ jobs:
iceberg_1_10:
name: Iceberg Spark SQL Tests (Iceberg 1.10)
needs: changes
- # Main-only by default; PRs need the `run-iceberg-tests` label. Iceberg 1.11
+ # 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.
if: needs.changes.outputs.iceberg_1_10 == 'true'
uses: ./.github/workflows/iceberg_spark_test_reusable.yml
@@ -322,12 +349,9 @@ jobs:
java: 17
# ---------------------------------------------------------------------------
- # required_checks: one flat job that aggregates every other job's result, so
- # that `main` has a single name it can safely require.
- #
- # Nothing requires it yet. It is landing first, on its own, so that we can
- # watch it report on real pull requests before `.asf.yaml` names it in
- # `required_status_checks` for `main`.
+ # required_checks: the single context listed in `required_status_checks` for
+ # `main` in `.asf.yaml`, and therefore the only thing the merge queue waits
+ # on.
#
# None of the jobs above can be required directly, because the name a caller
# of a reusable workflow publishes depends on whether it ran:
diff --git a/.github/workflows/iceberg_spark_test_reusable.yml b/.github/workflows/iceberg_spark_test_reusable.yml
index fe1e4757cd..ae20b6d768 100644
--- a/.github/workflows/iceberg_spark_test_reusable.yml
+++ b/.github/workflows/iceberg_spark_test_reusable.yml
@@ -139,7 +139,7 @@ jobs:
rust-version: ${{ env.RUST_VERSION }}
jdk-version: ${{ inputs.java }}
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
path: native/target/release/
@@ -180,7 +180,7 @@ jobs:
steps:
- uses: actions/checkout@v7
- name: Download Iceberg shard inventories
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
# Download all attempts: a failed-job rerun retains earlier successful
# shards. The checker selects the latest inventory for each index.
@@ -207,7 +207,7 @@ jobs:
rust-version: ${{ env.RUST_VERSION }}
jdk-version: ${{ inputs.java }}
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
path: native/target/release/
@@ -242,7 +242,7 @@ jobs:
rust-version: ${{ env.RUST_VERSION }}
jdk-version: ${{ inputs.java }}
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-iceberg-${{ inputs.iceberg-full }}-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
path: native/target/release/
diff --git a/.github/workflows/pr_build_linux.yml b/.github/workflows/pr_build_linux.yml
index 58c04f406a..a019e9a721 100644
--- a/.github/workflows/pr_build_linux.yml
+++ b/.github/workflows/pr_build_linux.yml
@@ -517,7 +517,7 @@ jobs:
jdk-version: ${{ matrix.profile.java_version }}
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-linux
# Download to release/ since Maven's -Prelease expects libcomet.so there
@@ -562,7 +562,7 @@ jobs:
jdk-version: 17
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-linux
path: native/target/release/
@@ -617,7 +617,7 @@ jobs:
jdk-version: 17
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-linux
path: native/target/release/
diff --git a/.github/workflows/pr_build_macos.yml b/.github/workflows/pr_build_macos.yml
index 5731b2f5be..66a1b62ba3 100644
--- a/.github/workflows/pr_build_macos.yml
+++ b/.github/workflows/pr_build_macos.yml
@@ -269,7 +269,7 @@ jobs:
protoc-architecture: aarch_64
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-macos
# Download to release/ since Maven's -Prelease expects libcomet.dylib there
diff --git a/.github/workflows/spark_sql_test_reusable.yml b/.github/workflows/spark_sql_test_reusable.yml
index 52b9d1208c..2790855332 100644
--- a/.github/workflows/spark_sql_test_reusable.yml
+++ b/.github/workflows/spark_sql_test_reusable.yml
@@ -196,12 +196,12 @@ jobs:
rust-version: ${{env.RUST_VERSION}}
jdk-version: ${{ inputs.java }}
- name: Download native library
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: native-lib-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
path: native/target/release/
- name: Download JVM compile artifact
- uses: actions/download-artifact@v8
+ uses: ./.github/actions/download-artifact-retry
with:
name: jvm-compiled-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
- name: Extract apache-spark/ (sources + compiled output)
@@ -285,6 +285,8 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Download fallback log artifacts
+ # Not wrapped in download-artifact-retry: that local action needs a
+ # checkout, which this job deliberately skips.
uses: actions/download-artifact@v8
with:
path: fallback-logs/
diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py
index 9d5c0b3c48..0d30910f29 100644
--- a/dev/ci/check-ci-config.py
+++ b/dev/ci/check-ci-config.py
@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
-# Guards four CI invariants that are silent when broken:
+# Guards five CI invariants that are silent when broken:
#
# 1. Change-filter routing. dev/ci/compute-changes.py decides which heavy
# jobs run. A file that a job depends on but that no filter lists makes
@@ -45,6 +45,11 @@
# by `needs`, and make the forced `overwrite` on an upload retry delete
# a sibling's finished artifact.
#
+# 5. Local actions resolve from the workspace, so a `uses: ./.github/...`
+# in a job that skipped the checkout cannot be loaded at all. Jobs that
+# run only under an input or a label can carry that for a long time
+# before anyone runs them.
+#
# Run from the repository root: python3 dev/ci/check-ci-config.py
import importlib.util
@@ -93,31 +98,65 @@ ROUTING_CASES = [
([".mvn/maven.config"], BUILD_JOBS),
([".mvn/wrapper/maven-wrapper.properties"], BUILD_JOBS),
(["mvnw"], BUILD_JOBS),
- # The upload wrapper is used by every producer of a shared artifact.
+ # The artifact wrappers are used by every producer and consumer of a
+ # shared artifact. Without these, an edit confined to one of them routes
+ # to nothing at all and merges having been exercised by no consumer.
([".github/actions/upload-artifact-retry/action.yaml"], BUILD_JOBS),
+ ([".github/actions/download-artifact-retry/action.yaml"], BUILD_JOBS),
# 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"}),
]
# Event policy. Each case is (event, expected set of jobs allowed to run),
-# where "allowed" ignores path filters. Transcribed from the `if:` expressions
-# ci.yml carried before POLICY moved into compute-changes.py, so these pin the
-# pre-refactor behaviour rather than restating the new code.
-PR_TIER = {"build_linux", "build_macos", "benchmark", "spark_3_5", "spark_4_1", "iceberg_1_11"}
+# 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", "spark_4_1", "iceberg_1_11"}
+SPARK_OPT_IN = {"spark_3_4", "spark_3_5", "spark_4_0"}
ICEBERG_OPT_IN = {"iceberg_1_8", "iceberg_1_9", "iceberg_1_10"}
-ALL_JOBS = PR_TIER | ICEBERG_OPT_IN | {"docs", "spark_3_4", "spark_4_0"}
+BUILD_OPT_IN = {"build_macos", "benchmark"}
+QUEUE_TIER = PR_TIER | SPARK_OPT_IN | ICEBERG_OPT_IN | BUILD_OPT_IN
+ALL_JOBS = QUEUE_TIER | {"docs"}
POLICY_CASES = [
# A manual run may exercise anything.
({"name": "workflow_dispatch"}, ALL_JOBS),
- # Push to main runs every job, docs included: it is the only event that
- # may deploy the site.
- ({"name": "push"}, 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.
+ ({"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
+ # showing up here means every merge is paying for it twice.
+ ({"name": "push"}, {"docs", "build_linux"}),
# A plain pull request: the PR tier only. docs must never run here, and the
# 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.
+ (
+ {"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.
+ (
+ {"name": "pull_request", "action": "synchronize", "labels": ["run-macos-tests"]},
+ PR_TIER | {"build_macos"},
+ ),
+ (
+ {"name": "pull_request", "action": "synchronize", "labels": ["run-benchmark-check"]},
+ PR_TIER | {"benchmark"},
+ ),
+ (
+ {
+ "name": "pull_request",
+ "action": "labeled",
+ "label": "run-macos-tests",
+ "labels": ["run-macos-tests"],
+ },
+ {"build_macos"},
+ ),
# An opt-in label present on a pushed commit adds just that suite.
(
{"name": "pull_request", "action": "synchronize", "labels": ["run-spark-3.4-tests"]},
@@ -168,12 +207,20 @@ POLICY_CASES = [
# `uses:` values that publish an artifact, and the one that consumes it.
UPLOAD_USES = re.compile(r"uses:\s*(\./\.github/actions/upload-artifact-retry|actions/upload-artifact@)")
-DOWNLOAD_USES = re.compile(r"uses:\s*actions/download-artifact@")
+DOWNLOAD_USES = re.compile(r"uses:\s*(\./\.github/actions/download-artifact-retry|actions/download-artifact@)")
# The artifact name is the first `name:` key of the step's `with:` block. A
# following step starts with `- `, which distinguishes it from a `with:` key.
WITH_NAME = re.compile(r"^\s+name:\s*(\S.*?)\s*$")
NEW_STEP = re.compile(r"^\s*-\s")
+# A job id in a workflow file, and the two `uses:` shapes the checkout guard
+# below cares about. `./.github/workflows/` is deliberately not matched: that
+# is a reusable-workflow call, which resolves from the repository rather than
+# from the runner's workspace and so needs no checkout.
+JOB_KEY = re.compile(r"^ ([A-Za-z0-9_-]+):\s*$")
+LOCAL_ACTION_USES = re.compile(r"uses:\s*(\./\.github/actions/\S+)")
+CHECKOUT_USES = re.compile(r"uses:\s*actions/checkout@")
+
def load_filters():
spec = importlib.util.spec_from_file_location("compute_changes", "dev/ci/compute-changes.py")
@@ -286,6 +333,41 @@ def check_artifact_names():
return not failures
+def check_local_actions_have_checkout():
+ """Every `uses: ./.github/actions/...` needs a checkout earlier in its job.
+
+ A local action is loaded from the runner's workspace, not from the
+ repository, so a job that has not checked out simply cannot find it. The
+ failure is at step level and only on the jobs that skipped the checkout,
+ which is easy to miss when those jobs are conditional.
+ """
+ failures = []
+ for path in sorted(WORKFLOWS.glob("*.y*ml")):
+ job = None
+ checked_out = False
+ for line in path.read_text(encoding="utf-8").splitlines():
+ if line.lstrip().startswith("#"):
+ continue
+ match = JOB_KEY.match(line)
+ if match:
+ job = match.group(1)
+ checked_out = False
+ continue
+ if CHECKOUT_USES.search(line):
+ checked_out = True
+ continue
+ match = LOCAL_ACTION_USES.search(line)
+ if match and not checked_out:
+ failures.append(
+ f"{path}: job `{job}` uses the local action {match.group(1)} "
+ f"with no preceding actions/checkout. Add the checkout, or "
+ f"call the underlying published action directly"
+ )
+ for failure in failures:
+ print(f"local action: {failure}")
+ return not failures
+
+
def ci_jobs_and_aggregator():
"""Return (all job ids in ci.yml, aggregator `needs:` ids, aggregator display name).
@@ -464,6 +546,7 @@ if __name__ == "__main__":
ok = check_change_filters()
ok = check_event_policy() and ok
ok = check_artifact_names() and ok
+ ok = check_local_actions_have_checkout() and ok
ok = check_required_checks() and ok
if not ok:
sys.exit(1)
diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py
index 0eb4cc2446..9716ced234 100644
--- a/dev/ci/compute-changes.py
+++ b/dev/ci/compute-changes.py
@@ -57,6 +57,7 @@ FILTERS = {
".github/actions/java-test/**",
".github/actions/rust-test/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
"!**.md",
"!native/core/benches/**",
"!native/spark-expr/benches/**",
@@ -80,6 +81,7 @@ FILTERS = {
".github/actions/setup-macos-builder/**",
".github/actions/java-test/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
"!**.md",
"!native/core/benches/**",
"!native/spark-expr/benches/**",
@@ -126,6 +128,7 @@ FILTERS = {
".github/actions/setup-builder/**",
".github/actions/setup-spark-builder/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -151,6 +154,7 @@ FILTERS = {
".github/actions/setup-builder/**",
".github/actions/setup-spark-builder/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -176,6 +180,7 @@ FILTERS = {
".github/actions/setup-builder/**",
".github/actions/setup-spark-builder/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -201,6 +206,7 @@ FILTERS = {
".github/actions/setup-builder/**",
".github/actions/setup-spark-builder/**",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -224,6 +230,7 @@ FILTERS = {
"dev/ci/check-iceberg-shards.py",
"dev/ci/test-iceberg-shards.py",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -247,6 +254,7 @@ FILTERS = {
"dev/ci/check-iceberg-shards.py",
"dev/ci/test-iceberg-shards.py",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -270,6 +278,7 @@ FILTERS = {
"dev/ci/check-iceberg-shards.py",
"dev/ci/test-iceberg-shards.py",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -293,6 +302,7 @@ FILTERS = {
"dev/ci/check-iceberg-shards.py",
"dev/ci/test-iceberg-shards.py",
".github/actions/upload-artifact-retry/**",
+ ".github/actions/download-artifact-retry/**",
".mvn/**",
"mvnw",
],
@@ -301,6 +311,7 @@ FILTERS = {
# Which events may run each job, independent of the path filters above.
#
# "pr" every pull request
+# "queue" the merge queue, i.e. a merge_group event
# "push" push to main
# "label:<name>" a pull request carrying that label
#
@@ -308,21 +319,43 @@ FILTERS = {
# "label:" are mutually exclusive -- a job is either unconditional on pull
# 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.
POLICY = {
- "build_linux": ["pr", "push"],
- "build_macos": ["pr", "push"],
- "benchmark": ["pr", "push"],
- # docs deploys to asf-site, so it must not run from a pull request.
+ # 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
+ # own branch or on main, and nowhere else. The queue runs on a throwaway
+ # gh-readonly-queue/* branch, so whatever it saves is deleted with that
+ # branch. Without a push run, a Cargo.lock or pom.xml change would leave
+ # main's cargo-registry, Maven and TPC-H/TPC-DS caches stale forever, and
+ # every later pull request would pay the delta on top of the restore-keys
+ # prefix match.
+ "build_linux": ["pr", "queue", "push"],
+ # 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.
+ "build_macos": ["queue", "label:run-macos-tests"],
+ # Benchmark sources are compiled and linted, never run, so a break there
+ # cannot affect a PR's correctness verdict; the queue catches it.
+ "benchmark": ["queue", "label:run-benchmark-check"],
+ # 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": ["push", "label:run-spark-3.4-tests"],
- "spark_3_5": ["pr", "push"],
- "spark_4_0": ["push", "label:run-spark-4.0-tests"],
- "spark_4_1": ["pr", "push"],
- "iceberg_1_8": ["push", "label:run-iceberg-tests"],
- "iceberg_1_9": ["push", "label:run-iceberg-tests"],
- "iceberg_1_10": ["push", "label:run-iceberg-tests"],
+ "spark_3_4": ["queue", "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.
+ "spark_4_1": ["pr", "queue"],
+ "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", "push"],
+ "iceberg_1_11": ["pr", "queue"],
}
@@ -344,6 +377,8 @@ def event_allows(job, event):
return True
if name == "push":
return "push" in tiers
+ if name == "merge_group":
+ return "queue" in tiers
if name != "pull_request":
return False
diff --git a/docs/source/contributor-guide/iceberg-spark-tests.md b/docs/source/contributor-guide/iceberg-spark-tests.md
index 4b0e827333..4aef43b102 100644
--- a/docs/source/contributor-guide/iceberg-spark-tests.md
+++ b/docs/source/contributor-guide/iceberg-spark-tests.md
@@ -101,9 +101,11 @@ diff must be generated against its own tag.
The `iceberg_spark_test_<version>.yml` workflows apply these diffs and run the three Gradle targets above
against each Iceberg version. Iceberg 1.8.1 runs against Spark 3.4.3 with Java 11; Iceberg 1.9.1 and 1.10.0
run against Spark 3.5.9 with Java 17; Iceberg 1.11.0 runs against Spark 4.1.3 with Java 17. Iceberg 1.11
-(the only version testing Spark 4.1) runs on every pull request and on pushes to main; the older versions
-(1.8, 1.9, 1.10) run only on pushes to main, or on a pull request labeled `run-iceberg-tests`. All caller
-workflows delegate to `iceberg_spark_test_reusable.yml`, which holds the build and test job logic.
+(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
+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.
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org