You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/14 16:17:16 UTC
[PR] chore: run only the cache-writing jobs on push to main [datafusion-comet]
andygrove opened a new pull request, #5930:
URL: https://github.com/apache/datafusion-comet/pull/5930
## Which issue does this PR close?
Closes #5929.
## Rationale for this change
`ci.yml` runs `pr_build_linux` on push to main, and the reason is
`actions/cache` scoping rather than coverage: a pull request can only restore
caches saved on its own branch or on `main`, and the merge queue builds on a
throwaway `gh-readonly-queue/*` branch whose caches are deleted with it. Without
a push run, main's caches go stale and every later pull request pays the delta.
It has been doing that by running the entire pipeline. Averaged over four recent
push-to-main runs, `pr_build_linux` costs **587 runner-minutes** a push, and all
but ~73 of that is lints, the 5x4 `linux-test` matrix and the TPC-H/TPC-DS query
passes — none of which test anything new, because the queue already ran the same
jobs against the exact tree that landed.
| job group | avg min/push | writes a cache main needs? |
| ------------------------------------ | -----------: | ----------------------------------- |
| `linux-test` (5 profiles x 4 suites) | 456.1 | no |
| `Build Native Library` | 32.3 | yes, `cargo-ci` |
| `Verify TPC-DS Results` | 26.4 | yes, dataset + `java-maven` |
| `ubuntu-latest/rust-test` | 23.7 | yes, `cargo-debug` |
| `Lint Java` (matrix) | 22.0 | no, prefix-shares `java-maven` |
| `Celeborn` (2 versions) | 10.8 | no, prefix-shares `java-maven` |
| `Verify TPC-H Results` | 10.4 | yes, dataset + `java-maven` |
| `Build Spark 4.1, JDK 17` | 4.5 | no, prefix-shares `java-maven` |
| `Lint` | 0.7 | no, but both native jobs `needs:` it |
| `Lint Scala (syntactic)` | 0.5 | no |
| **total** | **587.4** | |
At the 8-10 pushes a day this repository sees, that is roughly 4,100-5,200
runner-minutes a day. For scale, all of `ci.yml` came to 1,257 runner-hours on
2026-09-12 (44,440 min on pull requests, 26,395 in the queue, 4,588 on push).
## What changes are included in this PR?
- **`pr_build_linux.yml`** gains a `cache-refresh-only` boolean input. When set,
only the jobs that own an `actions/cache` entry run: `build-native`
(`cargo-ci`), `linux-test-rust` (`cargo-debug`), the two TPC-H/TPC-DS jobs
(the SF=1 datasets and the shared `java-maven` entry), and `lint`, which is 40
seconds and which both native jobs `needs:`. Everything else carries
`if: ${{ !inputs.cache-refresh-only }}`. Inside the two TPC jobs only the
query passes are skipped; data generation still runs, or the dataset caches
would never be written.
- **`compute-changes.py`** gains `build_linux_full`, tiers `["pr", "queue"]`,
sharing `build_linux`'s `FILTERS` by assignment. This is the existing
`spark_4_1` / `spark_4_1_hive` shape: two POLICY outputs feeding one call.
- **`ci.yml`** folds it in as
`cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' }}`.
- **`check-ci-config.py`** gains a sixth invariant, `check_cache_refresh_scope`:
every job in `pr_build_linux.yml` must be either listed in
`CACHE_REFRESH_JOBS` (with the cache entry it writes) or carry the guard, and
`ci.yml` must actually pass the input. Both failure modes are silent —
the runner bill goes back up and nothing turns red — which is why they need a
check rather than a comment.
- **`.github/workflows/README.md`** updated: the push-tier paragraph, the
diagram, the "What runs when" row, and a note in "Changing what runs when"
that an output need not map one-to-one onto a job.
Net effect: the push tier goes from 587 runner-minutes to about 73. Nothing
changes for pull requests, the merge queue, or `workflow_dispatch`, all of which
set `build_linux_full=true` and so get `cache-refresh-only: false`.
Why an input rather than a separate cache-warming workflow: the cache keys are
the entire point of the push tier, and a second workflow would have to repeat
every one of them. Keeping both modes in one file means they cannot drift.
Two deliberate consequences, both stated in the issue:
- The suffixed Maven keys (`-lint`, `-spark-4.1-build`, `-celeborn-*`) stop
being refreshed on main and fall back to the shared `<os>-java-maven-`
restore-keys prefix, which the TPC jobs still write. Cost is a small delta
download per lint job on a pull request.
- `<os>-cargo-registry-*` stops being written on main, because `linux-test` owns
it. Those jobs pass `skip-native-build: true` and run no cargo command, so the
entry is close to unused.
## How are these changes tested?
`dev/ci/check-ci-config.py` is the test, and it runs in `preflight` on every PR,
merge group and push. The new check was mutation-tested locally — each of these
fails it, and the unmodified tree passes:
| mutation | caught by |
| ---------------------------------------------------- | ----------------------------------------- |
| drop the guard from `linux-test` | `check_cache_refresh_scope` (job unguarded) |
| drop the `with:` block from `ci.yml` | `check_cache_refresh_scope` (input never passed) |
| guard `build-native`, a cache writer | `check_cache_refresh_scope` (cache goes stale) |
| put `build_linux_full` back on the `push` tier | `check_event_policy` (`POLICY_CASES`) |
Routing was verified by running `compute-changes.py` directly for each event
against a `spark/src/main/**` change:
| event | `build_linux` | `build_linux_full` | `cache-refresh-only` |
| ------------------ | ------------- | ------------------ | -------------------- |
| `push` | true | false | **true** |
| `merge_group` | true | true | false |
| `pull_request` | true | true | false |
| `workflow_dispatch`| true | true | false |
A docs-only push still routes to neither, so the workflow does not run at all.
`actionlint` and `prettier --check` pass. The behaviour on `push` itself can
only be confirmed after merge, on the first push to main — the check to make is
that `PR Build (Linux)` reports five jobs rather than 32.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: run only the cache-writing jobs on push to main [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5930:
URL: https://github.com/apache/datafusion-comet/pull/5930#issuecomment-5671799120
Merged `main` to clear a conflict with #5885. The conflict itself was small — that PR removed `spark_3_4` from `SPARK_OPT_IN` and added `SPARK_DEPRECATED` in the same list-shaped hunk this branch touches to add `build_linux_full` to `PR_TIER`, so the resolution is just both.
The more useful thing the merge turned up is that #5885 documents the push tier in `docs/source/contributor-guide/ci.md`, which I had missed — my original diff only updated `.github/workflows/README.md`. That guide said the Linux build "also runs on push so that the dependency caches on main stay fresh" with no mention that it is now cache-writers-only, which would have left the contributor-facing docs describing the old behaviour. I have added the cache-refresh-only paragraph there and a line about the new invariant in the "Changing CI itself" section. Same file also gets the guidance a contributor actually needs: if you add a job to `pr_build_linux.yml`, give it the guard unless it writes a cache main needs, and the config check will tell you if you forget.
I re-ran the four mutation cases after the merge rather than assuming they survived it, plus a fifth for #5885's own invariant (putting `spark_3_4` back in the queue), and re-checked the per-event routing table. All still hold.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: run only the cache-writing jobs on push to main [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #5930:
URL: https://github.com/apache/datafusion-comet/pull/5930
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org