ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue

[andygrove]

Which issue does this PR close?

Closes #5838.

Rationale for this change

CI load on pull requests is dominated by suites that don't really need to be green until merge time. A merge queue lets us move them there: the queue tests the exact tree that will land, so an iterating PR can run a smaller set without losing any coverage on main.

The part I expected to be a blocker turned out not to be. Merge queue is self-serve in .asf.yaml now. Rulesets there accept a raw GitHub Rulesets API payload, and asfyaml/feature/github/rulesets.py passes it through with no rule-type allowlist (_to_payload_ruleset returns the raw mapping after coercing the numeric keys), so a merge_queue rule needs no INFRA ticket. apache/iceberg-python, apache/seatunnel and apache/texera all run queues configured this way. Worth noting apache/datafusion is not the template even though it's the obvious place to look: their .asf.yaml has no queue config and gh api repos/apache/datafusion/rulesets returns [], because INFRA enabled theirs directly in classic branch protection via INFRA-27154 before asfyaml supported rulesets.

What changes are included in this PR?

CI splits into two tiers:

	PR tier	queue tier	push to main
build Linux	✓	✓	✓ (refreshes caches)
Spark 4.1, Iceberg 1.11	✓	✓	—
build macOS, benchmark check	label only	✓	—
Spark 3.4 / 3.5 / 4.0, Iceberg 1.8 / 1.9 / 1.10	label only	✓	—
docs deploy	—	—	✓

Three things change for contributors:

• Spark 3.5 ran on every PR and is now queue-only, with a run-spark-3.5-tests label as an escape hatch. Spark 4.1 stays in the PR tier because it's the default build profile, so it's the cheapest early warning that something is wrong.
• The macOS build is queue-only, with run-macos-tests as the escape hatch. macOS runners are the scarcest capacity we have, and the Linux build already covers rustfmt and the Rust/JVM compile on every PR, so the PR tier loses nothing but platform-specific coverage.
• The benchmark check is queue-only, with run-benchmark-check as the escape hatch. It only compiles and lints benchmark sources, so a break there can't change a PR's correctness verdict.

The Linux build keeps a push arm for one reason: actions/cache scoping. A PR 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, so something has to refresh main's cargo-registry, Maven and TPC-H/TPC-DS caches after a Cargo.lock or pom.xml change. This was @blaginin's suggestion in review.

Dropping the push arm from the other heavy jobs is load-bearing rather than cleanup. The queue already tested the 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, not down. Iceberg 1.8/1.9/1.10 move into the queue for the same reason. They were push-only, so otherwise they'd have nowhere left to run.

On the .asf.yaml side: a Merge Queue ruleset targeting ~DEFAULT_BRANCH (merge queue rules reject wildcard ref patterns, so release branches keep plain protection), Required Checks as the single required status check, and allow_auto_merge so a PR can be queued before its checks finish. apache/root is a bypass actor so a wedged queue can be recovered without a ticket. GitHub evaluates the ruleset and classic branch protection together, so the existing one-approving-review requirement is unaffected.

The queue parameter worth arguing about is max_entries_to_build: 2. I had the cost model wrong here at first and @sunchao caught it: merge limits do not combine merge_group builds, so max_entries_to_merge: 5 saves no CI at all. Every entry gets its own build of itself plus everything ahead of it in the queue, and that limit only caps how many already-green entries land in one merge operation. Build concurrency is therefore the throughput ceiling: two pipelines at the ~2.5h we see today drains roughly 19 merges a day, against a median of 6 and a busiest day of 22 over the last month on main. I have left it at 2 with those numbers written into the .asf.yaml comment, since raising it costs that many concurrent ASF runners, but it is a one-line change if we would rather start at 3. None of this changes the case for the split — the saving comes from the PR tier being small, not from batching inside the queue.

How are these changes tested?

A download-artifact-retry action mirrors the existing upload wrapper, after a ListArtifacts 403 killed a Spark 4.1 shard on this PR before it ran a single test. Every download that has a checkout uses it; merge-fallback-logs stays on the published action because it deliberately skips checkout. check-ci-config.py now holds both halves of that in place, after @sunchao found the wrapper was missing from the ten consuming path filters and that merge-fallback-logs had been switched to the local action anyway. The wrapper is registered in those filters with a ROUTING_CASES entry, and a new fifth invariant rejects any uses: ./.github/actions/... in a job with no preceding actions/checkout. I confirmed each guard fires by reintroducing the bug it catches.

preflight runs dev/ci/check-ci-config.py, which (since #5842) enforces that every job is registered with the aggregator and that the Required Checks name matches the context this PR adds to .asf.yaml. Since .asf.yaml now declares that context, the name-match half of the guard is live for the first time on this PR. I confirmed it fires by renaming the job locally. actionlint, prettier --check "**/*.md" and a YAML parse of both changed configs are clean.

Since #5850 moved the routing policy into compute-changes.py, the tier change here is a POLICY table edit covered by POLICY_CASES, rather than ten rewritten YAML expressions. Those cases are written out longhand rather than derived from POLICY, so every tier move has to be stated twice. I confirmed the check fails if macOS is put back in the PR tier. ci.yml only gains the merge_group trigger and the SHAs to diff a queue branch against.

The merge_group paths can't really be exercised until the ruleset is live. #5842 landed Required Checks on its own first so we could confirm it reports correctly on real PRs.

Notes for reviewers

• run-macos-tests and run-benchmark-check need creating in repository settings before this merges. The run-spark-*-tests and run-iceberg-tests labels already exist. A contains() on a label nobody can add is simply always false, so a missing label makes the escape hatch look like it silently does nothing.
• Once these suites gate the queue, a flaky test blocks everyone's merges rather than one PR. datafusion sent a dev@ note before enabling theirs and we should probably do the same.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B

[blaginin]

(for a pr on top) - let's run this on push to main? So will keep updating the cache...

[andygrove]

Thanks @blaginin. I went ahead and addressed this in the current PR

[comphead]

Thanks @andygrove I'll check this today

[sunchao]

Correctness

Reviewed e106db138b5a2f439a8d688131d154f25df1ab7a against f29a236128b3c0b175bb77b5b1f5192356c3056a. This moves macOS, benchmark compilation and Spark 3.5 out of the default PR tier, moves the older Spark/Iceberg suites from post-merge runs into the queue, and keeps Linux builds on push to refresh main's caches. The queue event diffs its base and head, then applies the existing path filters and the new event-policy table.

Two P2 issues remain in the artifact-download changes: the new shared action is missing from all consuming path filters, and the optional fallback-log merge job invokes it without checking out the repository. The inline comments describe their separate triggers. The latter does not affect current umbrella calls, which leave collect-fallback-logs false.

No Spark expression, operator, test contents or engine semantics change here. Spark 3.4/3.5/4.0/4.1 and Iceberg coverage is being rescheduled; this source review does not establish runtime semantic equivalence. Local configuration checks passed, and independent YAML/routing checks confirmed both gaps with in-memory corrective controls. The required-name guard also rejected a renamed context, a missing dependency and reuse of the required name on label runs.

Preflight passed, including the configuration guard and actionlint. Its checkout was 27a801114a82e356ea988bc76640fa4b33f4e0a1, with the reviewed base/head as parents and the same tree as this head. The CI run was still in progress when checked. No local native/JVM suites or live merge-queue enforcement tests were run.

Performance

Deferring these suites reduces repeated work while a PR is being revised. Keeping only Linux and docs on push also avoids repeating most queue work after landing.

The stated five-PRs-per-pipeline saving needs correction: GitHub documents that max_entries_to_merge controls landing limits after checks pass and does not combine merge_group builds. With ALLGREEN, successive entries still require their own successful queue validations. Build concurrency limits simultaneous groups, not the number of validations charged to a batch. No measured queue throughput or runner-minute reduction is established here. GitHub merge-queue behavior

Design

Keeping path selection and event policy separate makes the tier changes easy to inspect. The existing flat aggregate remains the required context, while label-triggered runs publish a separate name; docs deployment stays outside the merge decision. These choices preserve a stable status name without forcing every reusable-workflow matrix name into branch protection.

The ASF raw-ruleset handler supports this payload shape and coerces its numeric parameters. The read-only ruleset list and public branch metadata returned no rulesets and no required status contexts. Full protection settings were not available from the protection endpoint, so this is proposed configuration, not a verified rollout. The macOS and benchmark opt-in labels were already present when checked.

Abstraction & complexity

The small download wrapper centralizes bounded retries without changing the callers' artifact names or extraction paths. Its final attempt still propagates failure, and the explicit string default for merge-multiple preserves the underlying action's boolean-input contract. Registering this shared action alongside the upload action in routing checks, and retaining the upstream action in the checkout-free fallback job, would close the two concrete integration gaps without adding another abstraction.

[sunchao]

Correctness

[P2] Register the new shared action in the consuming path filters. FILTERS lists upload-artifact-retry/** but never download-artifact-retry/**. For an edit confined to this new action, the current compute() returns every output false on both a PR update and merge_group, so none of its Linux/macOS/Spark/Iceberg consumers exercise the change. The existing config guard also passes that configuration. Add this path to the ten consuming filters and a corresponding ROUTING_CASES entry, as for the upload wrapper. I verified that adding only those dependencies in memory restores all ten queue routes.

[andygrove]

Fixed in 049fa83. .github/actions/download-artifact-retry/** now sits in the same ten filters that already listed the upload wrapper, with the matching ROUTING_CASES entry beside it. Dropping it from one filter fails the check with expected iceberg_1_11=true, got false, so it can't quietly go missing again.

[sunchao]

Correctness

[P2] Keep this download usable without a checkout. When a caller enables collect-fallback-logs, this job starts on its own runner and its first step now references ./.github/actions/download-artifact-retry, but no step has checked out that directory. The action cannot be loaded, so the merged fallback-log artifact is no longer produced. The preceding comment describes the intended exception, but the uses value does the opposite. Restore actions/download-artifact@v8 here, or add a checkout before the local action. Current umbrella callers leave the option false, which explains why ordinary CI does not exercise this regression. GitHub's local-action requirement

[andygrove]

Good catch, and the worst combination: the comment was right and the uses: was wrong. Restored actions/download-artifact@v8 in f36626d.

Since no caller sets collect-fallback-logs, a comment was never going to hold that line, so I also added a fifth invariant to check-ci-config.py — any uses: ./.github/actions/... in a workflow needs an actions/checkout earlier in the same job. Reintroducing the bad uses: now fails preflight naming exactly that step.

[andygrove]

Thanks @sunchao. You're right about the queue parameters, and it's worse than a wording problem — GitHub says outright that merge limits do not combine merge_group builds, so max_entries_to_merge: 5 saves no CI whatsoever. I had the cost model backwards. .asf.yaml, the workflows README and the PR description all asserted the batching; all three are corrected in 6cacd83.

While rewriting those comments I noticed the ALLGREEN one was describing HEADGREEN's behaviour — "a batch is only merged if the combined tree is green" is the green-head rule, not the all-green one. Fixed that too.

The correction also moves where the real constraint sits. With every entry getting its own build, max_entries_to_build: 2 is the throughput ceiling rather than a speculation-cost knob: two pipelines at the ~2.5h we see today drains roughly 19 merges a day. Over the last 30 days main took a median of 6 merges a day but 22 on the busiest, so 2 would have been the bottleneck on the worst day. I've left it at 2 and written those numbers into the comment rather than changing the value, since raising it costs that many concurrent ASF runners and that felt like a call to make deliberately rather than fold into this PR. Happy to start at 3 if you think that's the safer default.

The case for the split itself is unaffected: the saving comes from the PR tier being small, not from anything the queue batches.

And agreed on the rollout caveat — the ruleset and the merge_group paths are proposed configuration until it's live, which is why #5842 landed Required Checks on its own first so we could at least confirm the required context reports correctly on real PRs.

[comphead]

I checked the ASF side of this against the asfyaml source and GitHub's merge queue docs, and read the workflows at 6cacd83. The claims that would sink it all hold up:

• asfyaml/feature/github/rulesets.py forwards a raw rules list to GitHub untouched. The only rule-type inspection is the copilot_code_review overlap check, so merge_queue needs no INFRA ticket.
• It coerces exactly the merge-queue numeric keys plus actor_id (INFRA-27864). It silently leaves a non-integer alone, so a typo surfaces as a GitHub 422 rather than an asfyaml error.
• apache/root is team id 118420. The repo currently has zero rulesets, so reconcile_rulesets creates Merge Queue cleanly.
• merge_method: SQUASH matches enabled_merge_buttons.
• apache/iceberg-python is real prior art, tuned much more conservatively: max_entries_to_build: 1, max_entries_to_merge: 1, check_response_timeout_minutes: 90, no bypass actors.

Rollout risk

Rehearse merge_group on a scratch branch first. The path has never executed. If changes errors on a merge group under set -euo pipefail, required_checks goes red, every entry is evicted, and the fix has to merge through the broken queue. This is self-serve: land a ruleset targeting a literal refs/heads/mq-rehearsal with both merge_queue and required_status_checks: Required Checks. Merge-queue rules reject wildcards, not literal refs, and ci.yml has no branches: filter on pull_request or merge_group, so a throwaway PR against that branch exercises the whole path. A ruleset on a scratch branch cannot wedge main.

Make the merge_group diff fail open, as cheap insurance:

git diff --name-only "$MQ_BASE_SHA"..."$MQ_HEAD_SHA" > changed_files.txt || {
  echo "::warning::merge_group diff failed; running everything"
  git ls-tree -r --name-only "$MQ_HEAD_SHA" > changed_files.txt
}

Add a project-side bypass actor. apache/root still means an INFRA ticket. apache/datafusion-committers exists (id 9960695) and would let the PMC merge the fix directly. Ruleset bypass does not bypass classic branch protection, so the one-approval requirement still applies.

Gaps in the diff

The actions/cache exception was applied to Linux only. build_linux was kept on push for cache freshness, but it is not the only job writing caches nothing else writes:

Cache key	Written at	Only writer	Push tier after this PR
macOS-cargo-ci-v2-*	pr_build_macos.yml:89	build_macos	gone
macOS-cargo-registry-*	pr_build_macos.yml:280	build_macos	gone
macOS-java-maven-*	actions/java-test/action.yaml:70	build_macos	gone
Linux-spark-sql-*	actions/setup-spark-builder/action.yaml:55	spark_sql_test_reusable.yml	gone from all four callers
Linux-benchmark-maven-*	pr_benchmark_check.yml:63	benchmark	gone

The explicit saves are guarded on github.ref == 'refs/heads/main', false on a gh-readonly-queue/* branch, and the actions/cache@vN auto-saves land in a queue scope deleted with the branch. restore-keys means gradual degradation rather than a cliff, but Linux-spark-sql-* is the ~/.m2/repository for the suites now on the critical path of every merge, and a cold macOS cargo build on every queue run works against max_entries_to_build. Options: accept it and say so in POLICY, add "push" back to spark_4_1 on the same grounds, or add a push-only cache-warm macOS job without the test matrix.

In the queue, a fork PR's code runs with a write-capable GITHUB_TOKEN. A merge_group run is on a base-repo branch, so it gets the repo default token permissions and repository secrets, and actions/checkout persists that token into .git/config. Under pull_request those same suites get a read-only token and no secrets. docs.yaml pushes to asf-site with no permissions: block, which only works if the repo default is read and write, and neither ci.yml nor any reusable workflow it calls declares one. Cargo build scripts, Maven/sbt plugins and the applied Spark diff all run in that context. Fix is small, and also clears the CodeQL finding you hit on required_checks:

permissions:
  contents: read

jobs:
  docs:
    permissions:
      contents: write   # pushes to asf-site

Four workflows sit outside the umbrella and outside the queue. codeql.yml, delta_build_gate.yml and pyarrow_udf_test.yml trigger on pull_request and push: main with no merge_group. They are never evaluated against the merge result, are not in required_checks.needs, and are not required, so a red one does not stop a queueing. They also still run on push, which contradicts the no-push-tier rule stated two files away. Either fold them into ci.yml as reusables so they inherit the aggregator, or document them as advisory. check-ci-config.py only reasons about jobs inside ci.yml, so a new top-level workflow escapes the gate silently.

Tuning

check_response_timeout_minutes: 300 is calibrated on the PR tier. The queue tier adds three Spark SQL suites (each with its own ~40 minute build job), three Iceberg suites, macOS and the benchmark check. At max_entries_to_build: 2 that is roughly 150 concurrent jobs from this repo against a shared ASF pool, so wall clock is set by scheduling delay rather than fan-out. You can measure it without the queue: apply all six opt-in labels and then push a commit, since on synchronize the gates read from PR_LABELS and one run reproduces the queue tier exactly. Not workflow_dispatch though, since event_allows returns True unconditionally for it including for docs, and docs.yaml pushes to asf-site with no ref guard.

The cost model turns on the rebuild rate, not the tier sizes. Per merged PR with P CI-triggering pushes and B merge-group builds charged to it:

Δ = P × (macOS + benchmark + Spark 3.5) − (B − 1) × FULL − Linux-on-push

A full pipeline is several times macOS + benchmark + Spark 3.5, so every extra build charged to an entry eats several pushes' worth of savings. Probably still net negative at a typical P, but the margin is governed by B, so tracking evictions per week from day one is what decides whether this worked.

ALLGREEN is the right default, and there is a stronger argument for it than the comment gives. Under HEADGREEN with SQUASH, a group of two lands two commits on main and the first was never green on its own, which breaks bisect. The counterweight is that GitHub explicitly recommends HEADGREEN for flaky CI, and this repo has enough infra flakiness to justify retry wrappers on both upload and download. Worth naming the flip condition in the .asf.yaml comment, for example an eviction rate threshold over a week. One-line change either way.

Minor

• allow_update_branch: true becomes mostly redundant once the queue is live, and each click costs a full PR-tier rerun.
• Do not add strict: true later. Requiring branches to be up to date fights the queue.
• check-ci-config.py could assert merge_method matches the single enabled entry in enabled_merge_buttons, since drift there breaks every merge.
• The build_linux comment says queue caches "are deleted with the branch". For the three explicit cache/save steps the queue never writes them at all, because of the refs/heads/main guard. Same outcome, different mechanism.
• Agree on the dev@ note. That is also where the eviction-and-requeue recovery path should be spelled out for contributors.

[andygrove]

CI failure is unrelated:

 - fall-back: oss data location scheme (oss.* properties are not forwarded) (92 milliseconds)
  #
  # A fatal error has been detected by the Java Runtime Environment:
  #
  #  SIGSEGV (0xb) at pc=0x00007fb9d00e5800, pid=1201, tid=4733
  #
  # JRE version: OpenJDK Runtime Environment Zulu11.90+19-CA (11.0.32+9) (build 11.0.32+9-LTS)
  # Java VM: OpenJDK 64-Bit Server VM Zulu11.90+19-CA (11.0.32+9-LTS, mixed mode, tiered, compressed oops, g1 gc, linux-amd64)
  # Problematic frame:
  # C  0x00007fb9d00e5800
  #
  # Core dump will be written. Default location: Core dumps may be processed with "/usr/lib/systemd/systemd-coredump %P %u %g %s %t 9223372036854775808 %h %d" (or dumping to /__w/datafusion-comet/datafusion-comet/spark/core.1201)
  #
  # An error report file with more information is saved as:
  # /__w/datafusion-comet/datafusion-comet/spark/hs_err_pid1201.log
  #
  # If you would like to submit a bug report, please visit:
  #   http://www.azul.com/support/
  #