ci: run the non-default Spark and Iceberg suites nightly instead of in the merge queue - #5963
Conversation
Leave the Linux build as the whole PR tier. Spark 4.1 (catalyst and sql_core) and Iceberg 1.11 were the last test suites still running on every push to every pull request, and with agent-driven review and agent-driven replies to review a pull request now goes through several more push rounds before it is queued. The queue runs both suites against the merge result before anything lands regardless. `run-spark-4.1-tests` now gates the whole Spark 4.1 call, so it appears on both of that job's POLICY outputs; `run-spark-4.1-hive-tests` keeps selecting only the hive shards. `run-iceberg-tests` opts a pull request into all four Iceberg versions. Pin the new routing in check-ci-config.py and update the workflows README and the contributor guide. Closes apache#5938
…on pull requests The five Spark profiles in the linux-test matrix cost about the same each and together are three quarters of what the Linux build spends on a pull request. Run the Comet test suites against Spark 4.1 only on a pull request and against every profile in the merge queue, or on a pull request carrying the new `run-all-spark-profiles` label. The Lint Java matrix still compiles Spark 3.4/3.5/4.0 on every pull request, so a shim that fails to build is still caught there; only the runtime suites wait for the queue. A job-level `if:` cannot see `matrix`, so the profile rows move out of the workflow into dev/ci/linux-test-profiles.py with a tier each. The `lint` job runs it with a new `profiles` input and publishes the rows as a job output that the matrix reads with `fromJSON`, the same way the Spark SQL reusable workflow selects its module shards. A third POLICY output, `build_linux_all_profiles`, feeds that input from ci.yml; a `labeled` run sets only that output and runs just the profiles the PR tier skipped. check-ci-config.py asserts the two tiers partition the list, that the `pr` tier is exactly the default profile, and that ci.yml passes the input.
Review feedback: the README should describe the current tiers, not how they got there. The Lint Java sentence stays because it is current state.
…n the merge queue The merge queue ran three Spark SQL versions, four Iceberg versions and the Linux test matrix against every Spark profile on each merge. Spark 3.5, Spark 4.0 and Iceberg 1.8/1.9/1.10 were about 870 of the 1,900 runner-minutes a queue run cost, and the old Iceberg versions were the most common reason a queue run went red on a tree that was fine. Add a "nightly" tier to POLICY, fired by a schedule trigger on ci.yml at 06:00 UTC. The queue keeps one Spark version (4.1, the default profile) and one Iceberg version (1.11, the only Spark 4.1 coverage). Everything else moves to the nightly run: Spark SQL on 3.5 and 4.0, Iceberg 1.8/1.9/1.10 and the linux-test matrix's non-default Spark profiles. Their run-* labels still opt a pull request in early. The nightly run tests main as it stands, so it skips itself when nothing has landed since the previous one. A red nightly has no pull request to appear on, so a nightly_report job opens an issue labelled ci-nightly-failure listing the failed jobs, or comments on the one already open. The linux-test profile selector queue-only is renamed nightly, since those rows no longer run in the queue. check-ci-config.py gains a NIGHTLY_TIER set and a schedule case so a suite drifting back into the queue fails preflight.
github-actions
Bot
added
build
|
cc @blaginin |
| head_ts=$(git log -1 --format=%ct) | ||
| age=$(( $(date +%s) - head_ts )) | ||
| if [[ "$age" -gt $(( 24 * 3600 + 1800 )) ]]; then | ||
| echo "HEAD is $(( age / 3600 ))h old; nothing has landed since the last nightly" | ||
| export NIGHTLY_STALE=true |
There was a problem hiding this comment.
works as a first version, but in the future we maybe can use github api to check past runs explicitly?
There was a problem hiding this comment.
Done in 8533d0c: dev/ci/nightly-base.py now looks up the head sha of the last successful scheduled run through the Actions API and the nightly diffs against that, so every commit is covered by exactly one nightly and a red night keeps its commits in scope until a green one supersedes it. The time window is only the fallback for the first run, an API error, or a base that is no longer on main.
…iles nightly Diff the scheduled run against main's tip as of the previous tick instead of treating it as an event with no base. The nightly then goes through FILTERS like every other event: a quiet day or a docs-only day routes to nothing, and the NIGHTLY_STALE handshake and the schedule branch in compute-changes.py go away. With profiles nightly, pr_build_linux.yml now runs only lint, build-native and the linux-test matrix. The lints, Rust tests, Spark build and TPC runs already produced their verdict at the same commit, in the queue and push run for the nightly and in the PR tier for a label run. check-ci-config.py pins that guard the same way it pins cache-refresh-only, and the Cargo cache save is limited to the push run. Spell out QUEUE_TIER longhand, reject a POLICY entry that lists both queue and nightly, key the concurrency subgroup on the event name, reduce the profile selector to one comprehension, and trim repeated comments.
Look up the head sha of the most recent successful scheduled ci.yml run through the Actions API and diff main against it, so every commit is covered by exactly one nightly and a red night keeps its commits in scope until a green one supersedes it. The 24h30m window stays as the fallback for the first run, an API error, or a base that is no longer on main.
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
Reviewed 8533d0cf08cdf813e20ba708c0a4df9f07b4d73b. The new last-green-run lookup fixes the normal path, but one P2 remains in the time-based fallback: an API failure can discard source changes that still need nightly coverage. The inline comment includes a reproduced example and two ways to preserve coverage. No other actionable findings across routing, Linux matrix/dependencies, gating, reporting, and documentation.
Validation:
- CI configuration checks and actionlint passed on this revision.
- Seven helper cases and five integrated helper/workflow/router cases were exercised locally. The integrated reproduction ran the actual helper against a local API server and the exact workflow shell: a successful lookup selected all six nightly outputs, while HTTP 503 selected none for the same history.
- Profile/dependency, schedule-routing, and reporting-script probes also passed for the unchanged portions.
CI status: this head currently reports only a successful label check. The CI suites have not run on this [skip ci] commit. Local validation did not execute a hosted scheduled workflow or create a failure issue.
| if [[ -z "$prev" ]] || ! git merge-base --is-ancestor "$prev" HEAD 2>/dev/null; then | ||
| prev=$(git rev-list -1 --before="24 hours 30 minutes ago" HEAD) |
There was a problem hiding this comment.
[P2] Preserve pending coverage when the nightly baseline is unknown
The successful API lookup now preserves coverage, but this fallback can still lose it. Could we run the full nightly tier when no trustworthy baseline is available, or fail change detection so the run cannot become a successful baseline?
For example, the last green nightly was 72 hours ago, a source change landed 60 hours ago, that change has not passed a nightly, and only documentation changed recently. If the Actions API returns HTTP 503, nightly-base.py warns and exits 0 with no SHA. This fallback selects the 60-hour-old source commit as prev, so the diff contains only documentation and all nightly jobs skip. The run can finish green. Once the API recovers, that new green HEAD becomes the baseline, so the pending source coverage has been permanently dropped unless another relevant change triggers it.
I reproduced this with the actual helper against a local API server, these workflow commands, and the current routing script. The normal API response selected all six nightly outputs. HTTP 503 returned exit 0 and selected none for the same Git history. This is the remaining error-path gap in the last-green-run update.
There was a problem hiding this comment.
Good catch, and thanks for the reproduction — that is exactly the failure. Fixed in 01d000f by taking your first option: when there is no base the run can trust, it lists the whole tree and runs the entire nightly tier. The time window is gone rather than widened, because any guessed base has the same shape of bug at some history, and a run that covered everything is a base tomorrow can rely on.
I ran your scenario against the previous revision to confirm it, then against the fix, using the real helper, a local API server and the bash from ci.yml:
=== BEFORE (time-window fallback), API down ===
::warning::could not list previous scheduled runs (HTTP Error 503: Service Unavailable)
Nightly base: uncovered source change
selected:
=== AFTER (whole-tree fallback), API down ===
Nightly base unknown; running the whole nightly tier
selected: build_linux_all_profiles spark_3_5 spark_4_0 iceberg_1_8 iceberg_1_9 iceberg_1_10
The other three paths still behave: a successful lookup diffs against it and selects the same six, a lookup that returns HEAD (a quiet day) selects nothing, and a base that has left main takes the fallback. Note that the whole-tree list goes through FILTERS and POLICY unchanged, so it can only ever reach the nightly tier — nothing in the queue tier comes back.
Since the thing that went wrong here is silent, I also added check_nightly_base_fallback to check-ci-config.py. It pins the schedule case to the nightly-base.py lookup and the whole-tree fallback, and rejects a base derived from the clock (--before/--since/--after/--until) with your scenario as the error message. Mutation-tested three ways: restoring the time window, replacing the lookup with a fixed revision, and renaming the case all fail the check.
The scheduled run's base is the commit the last successful scheduled run tested. When that lookup yields nothing -- the first nightly, an Actions API error, or a base no longer on main -- it fell back to the tip as of 24 hours ago. That window can silently drop coverage: if the last green nightly was three nights ago and an uncovered source change landed two nights ago, a one-day window starts after that change, so its suites are skipped, the run goes green, and the green head becomes tomorrow's base. Fall back to listing the whole tree instead, the same way the first push to a branch does, so POLICY selects the entire nightly tier and a green run is a base that can be trusted. check-ci-config.py pins the shape of the schedule case, since the failure it prevents is silent.
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed 01d000fc against a8e8157e, following review 5212699804. The nightly-coverage P2 is fixed. An unknown or non-ancestor baseline now selects the entire nightly tier instead of guessing a time window. No new or remaining P1/P2 findings.
The same exact helper/workflow reproduction now selects all six nightly outputs after HTTP 503, where the previous revision selected none for the uncovered-source history. Ten baseline cases, event/label/profile checks, four negative checker mutations, and both failure-report JavaScript stub cases passed. The five original profile definitions remain unchanged. These are local component checks, not a hosted scheduled run or issue-publication test.
Preflight, including actionlint, passed. Required Checks failed on its cancelled Linux dependency, so overall CI remains incomplete. The tested merge preview c61a39cf used base 4479e722, not the assigned a8e8157e, but all changed workflow files match the reviewed head. No complete product-test pass or measured runner-time savings are claimed.
# Conflicts: # .github/workflows/README.md # .github/workflows/ci.yml # .github/workflows/pr_build_linux.yml # dev/ci/check-ci-config.py # dev/ci/compute-changes.py # dev/ci/linux-test-profiles.py # docs/source/contributor-guide/ci.md # docs/source/contributor-guide/development.md # docs/source/contributor-guide/iceberg-spark-tests.md # docs/source/contributor-guide/spark-sql-tests.md
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed 11b08035 against 347d8cf3, following review 5214150540. No new or remaining P1/P2 findings. The merge of main preserves all 11 authored workflow, routing, and documentation files byte for byte from the approved revision. PR #5939 is now merged into this base, and the nightly-coverage fallback remains fixed.
The CI configuration check and 28 event/label/input checks passed locally. The five profile definitions remain unchanged and split into one default and four nightly profiles. The prior ten-case helper/workflow reproduction still applies to identical source. It was not rerun. Current Preflight, including actionlint, passed on merge preview 431ca295, whose parents are this exact base/head and whose full tree matches the reviewed head. Native build and Rust checks passed, and the completed shuffle job verified the native artifact's digest. Overall CI is still running. A hosted scheduled run, failure-issue publication, and runner-time savings remain unvalidated.
Four conflicts, all in the CI tiering that landed on main while this branch was out (apache#5926, apache#5939, apache#5963): - dev/ci/compute-changes.py: keep both the spark_4_2 FILTERS entry and main's spark_4_1_hive entry, and give spark_4_2 the POLICY entry it never had. compute() iterates FILTERS and calls event_allows(), which indexes POLICY, so a FILTERS key with no POLICY entry made the changes job raise KeyError: 'spark_4_2' on every event that is not a dispatch. The 4.2 filter list also picks up the inputs spark_4_1 gained since (spark-sql-modules.py, the artifact retry actions, .mvn and mvnw) so the shared-build routing cases cover it. - .github/workflows/ci.yml: expose both new outputs; take main's Detect changes script, whose schedule case replaces the dispatch key list; keep main's docs if: but keep this branch's permissions: contents: write, which the new top-level contents: read default makes load-bearing for the asf-site deploy. The spark_4_2 job's event check moves into POLICY, and the job joins required_checks.needs so it cannot fail without blocking the queue. - dev/ci/check-ci-config.py: declare spark_4_2 in BUILD_JOBS, in a SPARK_EXPERIMENTAL tier set feeding ALL_JOBS, and in POLICY_CASES. - .github/workflows/README.md: take main's tables and add the 4.2 rows. Spark 4.2 keeps its on demand only routing, now expressed as label:run-spark-4.2-tests (the label already exists on the repo) rather than an event check in ci.yml. adding_a_new_spark_version.md described the old dispatch gating, a key list main deleted, and a preflight label allowlist that no longer exists, so its CI section is rewritten to match. check-ci-config.py, actionlint and prettier all pass.
An actions/cache entry is scoped to the ref that wrote it: a run restores from its own ref and from the default branch, and nothing else. So an entry written from refs/pull/*/merge is visible only to another run of the same pull request, and one written from the merge queue's gh-readonly-queue/* branch is visible to nobody, because the queue deletes that branch when it is finished with it. Both still consume the repository's shared cache budget, which is evicted least-recently-used. On 2026-09-15 the repository held 12.27 GiB across 14 entries: 9.22 GiB on one gh-readonly-queue/* branch, 3.01 GiB on refs/pull/*/merge, and nothing at all on main. Seven near-identical Linux-java-maven-* Maven repositories accounted for 9.06 GiB of it, every one of them a write-only copy. With main's cargo-ci entry evicted, all eight native builds in merge_group run 34977717057 missed their cache and paid a cold compile of 20 to 28 minutes each, 259 runner-minutes in a single run. The same build on the push run two hours later restored in 56s and compiled in 2m21s. The two cargo saves in pr_build_linux.yml carried `if: github.ref == 'refs/heads/main'` when this was written. #5963 has since replaced that guard with `if: github.event_name == 'push'`, which also excludes the scheduled run at the same sha, so both conditions are now spelled out together and check_cache_save_scope can see the ref half. Hold every other large cache to the same rule: a cache of ~/.m2/repository or of a cargo tree is restored everywhere and saved only on push to main. The bare actions/cache@vN form cannot express that, since it saves in an implicit post step no `if:` can reach, so those sites split into actions/cache/restore plus a guarded actions/cache/save. The TPC-H and TPC-DS datasets keep the read-write form: they are a few hundred MB, they are keyed on the workflow file, and a pull request editing that file would otherwise regenerate them on every run. publish_snapshot.yml keeps it too, since it runs from main on a schedule already. check_cache_save_scope in dev/ci/check-ci-config.py pins both halves, and rejects a new bare actions/cache@vN on any of those paths. Six mutations were used to confirm it fails: reverting a site to the read-write form, dropping the github.ref guard from each of four saves, and adding a fresh unguarded cargo cache. pyarrow_udf_test.yml now lists itself in its paths filter. It sits outside the ci.yml umbrella, so compute-changes.py does not route it, and an edit to the file merged without the workflow ever running -- including the cache step changed here.
3 participants
Which issue does this PR close?
Follow-on to #5870 and #5938. Stacked on #5939: this PR shows that PR's three commits until it merges.
Rationale for this change
Every merge-queue run currently executes three Spark SQL versions, four Iceberg versions and the Linux test matrix against all five Spark profiles. Sampling the last six queue runs on 2026-09-15:
Spark 3.5, Spark 4.0 and Iceberg 1.8/1.9/1.10 are about 870 of those minutes, roughly 45% of every queue run, and the analysis in #5870 found the old Iceberg versions were the most common reason the queue tier went red on a tree that was fine. The regressions those suites catch are real but rare, and a day's delay in seeing one costs less than paying for the suites on every merge.
What changes are included in this PR?
nightlytier inPOLICY(dev/ci/compute-changes.py), fired by ascheduletrigger onci.ymlat 06:00 UTC. The queue keeps one Spark version (4.1, the default build profile, all modules) and one Iceberg version (1.11, the only Spark 4.1 coverage). Spark SQL on 3.5 and 4.0, Iceberg 1.8/1.9/1.10 and thelinux-testmatrix's non-default Spark profiles move to the nightly tier. Their existingrun-*labels still opt a pull request in early.mainagainst the commit the last successful scheduled run tested (dev/ci/nightly-base.pylooks it up through the Actions API, so thechangesjob getsactions: read) and is routed by the sameFILTERSas every other event. Every commit is covered exactly once, and a red nightly keeps its commits in scope until a green one supersedes it. With no such run, an API error, or a base no longer onmain, there is no base it can trust, so it treats every tracked file as changed and runs the whole nightly tier. A day with no merges, or with only documentation changes, runs nothing.profiles: nightly,pr_build_linux.ymlruns onlylint,build-nativeand thelinux-testmatrix. The lints, Rust tests, Spark build, Celeborn checks and TPC-H/TPC-DS runs carryif: ${{ inputs.profiles != 'nightly' }}, since the queue and the push run already produced those verdicts at the same commit (and the PR tier did, for arun-all-spark-profileslabel run). The Cargo cache save is limited to the push run for the same reason.nightly_reportjob, on the scheduled event only and only whenrequired_checksis not green, opens an issue labelledci-nightly-failurelinking the run and listing the failed jobs, or comments on the one already open so consecutive red nights accumulate in one place. The label exists in the repository.mainnever cancels that commit's push run.linux-testprofile selectorqueue-onlyis renamednightly, and the profile rows' tier with it, since those rows no longer run in the queue.check-ci-config.pyspells outQUEUE_TIERandNIGHTLY_TIERlonghand, adds aschedulecase, rejects aPOLICYentry that lists bothqueueandnightly, and gainscheck_nightly_scope, which pins theprofiles != 'nightly'guard the same way the cache-refresh guard is pinned, andcheck_nightly_base_fallback, which pins theschedulecase to the last-green-run lookup and the whole-tree fallback and rejects a base derived from the clock.nightly_reportis exempt fromrequired_checks.needsfor the same reasondocsis.How are these changes tested?
python3 dev/ci/check-ci-config.pypasses, including the newschedulecase, themerge_groupcase now asserting the nightly suites are absent, the queue/nightly exclusivity rule and the nightly-scope guard check.apache/mainthroughEVENT_NAME=schedule python3 dev/ci/compute-changes.pyreports exactlybuild_linux_all_profiles,spark_3_5,spark_4_0,iceberg_1_8,iceberg_1_9andiceberg_1_10true; an empty diff reports nothing true.python3 dev/ci/linux-test-profiles.py --profiles nightlyreturns the four non-default profile rows, and an unknown selector is rejected.dev/ci/nightly-base.pyrun locally against the real API prints nothing (no scheduled run exists yet) and exits 0; against an unreachable endpoint it emits a::warning::on stderr and exits 0, so the bash fallback takes over. The same query withevent=merge_groupreturns a sha, which exercises the parsing path.schedulebranch's four paths were exercised end to end against a scratch repository and a local API server, running the exact bash fromci.ymland the real routing script. A successful lookup selects the six nightly outputs; a lookup that returnsHEAD(a quiet day) selects none; an HTTP 503 and a base that has leftmainboth select the same six, and nothing outside the nightly tier. Reproducing @sunchao's case on the previous revision — last green nightly 72 hours back, an uncovered source change 60 hours back, documentation only since — selects nothing under HTTP 503, and six outputs after the change.check_nightly_base_fallbackwas mutation-tested: restoring the time window, replacing thenightly-base.pycall with a fixed revision, and renaming theschedulecase each makecheck-ci-config.pyfail with the reason.actionlint --shellcheck=offandprettier --checkpass on the edited files.nightly_reportjob cannot run before this lands, sincescheduleonly fires from the default branch and a manual dispatch skips it by design. I will watch the first scheduled run after merge.