Improve cache usage in CI #7678
Conversation
There was a problem hiding this comment.
Nice work @sarutak -- thank you very much
I will say I don't really understand how the cache action works / doesn't work for Rust projects. However, I did an unscientific spot check on PR CI timings for this change, and my conclusion is that these data show a significant CI timing improvement and thus we can / should merge this PR.
This PR:
| ./target/ | ||
| ./datafusion-cli/target/ | ||
| # this key equals the ones on `linux-build-lib` for re-use | ||
| key: cargo-cache-benchmark-${{ hashFiles('datafusion/**/Cargo.toml', 'benchmarks/Cargo.toml', 'datafusion-cli/Cargo.toml') }} |
There was a problem hiding this comment.
This cache key doesn't change unless we update the dependencies, right? What about the local code changes?
There was a problem hiding this comment.
Sorry for the late reply.
This cache key doesn't change unless we update the dependencies, right?
Yes.
What about the local code changes?
Local code change (except for Cargo.toml specified in hashFiles) doesn't affect the cache.
| # this key equals the ones on `linux-build-lib` for re-use | ||
| key: cargo-cache- | ||
| key: cargo-cache-benchmark-${{ hashFiles('datafusion/**/Cargo.toml', 'benchmarks/Cargo.toml') }} |
There was a problem hiding this comment.
The key above also includes 'datafusion-cli/Cargo.toml' -- is in intentional that these keys do not match?
key: cargo-cache-benchmark-${{ hashFiles('datafusion/**/Cargo.toml', 'benchmarks/Cargo.toml', 'datafusion-cli/Cargo.toml') }}
There was a problem hiding this comment.
Yes, it's intended.
This job (verify benchmark results) seems not to touch datafusion-cli, so this cache should not be stale on changing datafusion-cli/Cargo.toml.
* Improve cache usage in CI * Trigger GA to check the effect of improvement
Which issue does this PR close?
Closes #7677
Rationale for this change
Making the CI faster accelerate development.
What changes are included in this PR?
This PR includes improvement of cache usage in CI.
In the current workflow, the critical path seems
cargo checktoverify benchmark results (amd64). Soactions/cacheis used only for the jobs.Cached contents are also revised.
Are these changes tested?
Done by CI.
With my experiment, the total duration of workflow defined by
rust.ymlwas about 30 mins.https://github.com/sarutak/arrow-datafusion/actions/runs/6329626199
Are there any user-facing changes?
No.