andygrove opened a new pull request, #5973: URL: https://github.com/apache/datafusion-comet/pull/5973
## Which issue does this PR close? None; filed directly. ## Rationale for this change 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 that same pull request, and one written from the merge queue's `gh-readonly-queue/*` branch is visible to nobody at all, because the queue deletes that branch when it is finished with it. Both still count against the repository's shared cache budget, which is evicted least-recently-used. That is where the budget has been going. On 2026-09-15 the repository held 12.27 GiB across 14 entries: | Scope | entries | size | | ------------------------- | ------: | -------: | | one `gh-readonly-queue/*` | 9 | 9.22 GiB | | `refs/pull/*/merge` | 4 | 3.01 GiB | | `refs/heads/main` | 0 | 0 | Seven near-identical `Linux-java-maven-*` Maven repositories accounted for 9.06 GiB of it (`lint`, `spark-4.1-build`, `celeborn-0.6.0`, `celeborn-0.7.0`, `pyarrow-udf-4.0.4`, `pyarrow-udf-4.1.3`, `pyarrow-udf-4.2.0`), every one a write-only copy that nothing would ever restore. Nothing at all was on main, which is the only scope a future pull request can read. The cost lands on the native build. In merge_group run [34977717057](https://github.com/apache/datafusion-comet/actions/runs/34977717057) every one of the ten `Restore Cargo cache` steps completed in 1 second -- a straight miss -- and the eight Linux `cargo build --profile ci` jobs then paid a cold compile of 20 to 28 minutes each, 259 runner-minutes in a single run. Two hours later the push-to-main run [34992959611](https://github.com/apache/datafusion-comet/actions/runs/34992959611) restored in 56s and compiled in **2m21s**. Same build, same profile, 10x apart on nothing but cache state. Note this is orthogonal to sharing one native build across callers (#5841) and to moving suites nightly: those reduce how many cold builds there are and how often they run, this one decides whether a build is cold at all. It matters more, not less, once #5841 lands, because a single shared native build sits on the critical path of every Spark and Iceberg job. ## What changes are included in this PR? - Ten caches of a Maven repository or a cargo tree are now restored everywhere and saved only on push to main, matching the `if: github.ref == 'refs/heads/main'` guard the cargo caches already carried. The bare `actions/cache@vN` form cannot express that -- its save runs in an implicit post step no `if:` can reach -- so those sites split into `actions/cache/restore` plus a guarded `actions/cache/save`. Placement preserves today's semantics: the save is the last step, so a red job does not write, exactly as the implicit post step behaved. - `benchmark-maven-` and `spark-sql-` gain `java-maven-` as a second `restore-keys` entry. Both jobs are queue-only, so nothing writes their own prefix any more; `java-maven-` is what the TPC-H/TPC-DS jobs write on push to main, and a Maven repository is always safe to start from a superset or a subset of itself. - `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. Its path matching is on substrings rather than `~/.cargo/...` literals, because `publish_snapshot.yml` writes `${{ env.CARGO_HOME }}/registry`, which a literal would have missed. - The TPC-H and TPC-DS dataset caches keep the read-write form and are out of scope: `./tpch` and `./tpcds-sf-1` are a few hundred MB, are not dependency trees, and are keyed on the workflow file, so a pull request editing that file would regenerate the data on every run rather than once. `publish_snapshot.yml` is exempt in `CACHE_SAVE_SCOPE_EXEMPT`, since it runs from main on a schedule already. - A "Large caches are written on main only" section in the workflows README. The expected trade-off, stated plainly: jobs that only ever run on a pull request or in the queue no longer get a dedicated cache entry. They restore main's shared `java-maven` base through `restore-keys` and download their profile delta each run, which is what a cold pull request already did. I expect that to cost a minute or two per job against the ~184 runner-minutes recovered on the native builds, but I have not measured the delta -- the first queue run after this lands will show it. ## How are these changes tested? Rebased onto `4c2ab9686`. #5939 landed mid-review and moved `pyarrow_udf_test.yml` under the `ci.yml` umbrella; the conflict is resolved in favour of that form, and the `pyarrow_udf` entry in `FILTERS` already lists the workflow file, so the routing gap I had patched separately is already closed. Passing locally on this revision: - `python3 dev/ci/check-ci-config.py`, including the new check. The checker walks 17 large-cache sites across every workflow and composite action. - `check-suites.py`, `check-benchmark-runner.py`, `test-iceberg-shards.py`, `node --test dev/ci/pr-type-label.test.mjs`, `actionlint --shellcheck=off`, and `prettier --check` on the edited README. - Structural check that every `save` has a matching `restore` in the same job, that no job declares the `maven-cache` step id twice, and that every save referencing `steps.maven-cache.outputs` has that id declared. `check_cache_save_scope` was mutation-tested eight ways, each expected to fail and each observed to fail with the right message: reverting a site to the read-write form; dropping the `github.ref` guard from the `java-test`, `setup-spark-builder`, `rust-test` and `pr_build_linux` saves; adding a fresh unguarded cargo cache to a workflow; and removing each of the two `CACHE_SAVE_SCOPE_EXEMPT` entries, which confirmed both are live rather than dead config. Two further exempt entries for the TPC datasets turned out to be dead -- their paths were never in scope -- and were dropped. Not verified locally: the actual cache hit rate after this lands. That needs a push to main to write the first entries and a queue run afterwards to read them. I will check `actions/cache/usage` and the `Restore Cargo cache` step durations on the first few runs and report back on this PR. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
