Skip to content

Improve Ci cache #16709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alamb merged 15 commits into from
Jul 12, 2025
Merged

Improve Ci cache #16709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
26e1f74
Add rust CI cache
blaginin Jul 5, 2025
81d1418
comment save if for now
blaginin Jul 5, 2025
de4bb0a
Add cache to depscache
blaginin Jul 7, 2025
165b288
Unify the containers
blaginin Jul 7, 2025
8017c92
linux-build-lib to use its own cache
blaginin Jul 7, 2025
46484e4
rollback datafusion-cli for now (blocked by https://github.com/apache…
blaginin Jul 7, 2025
d5b0eaa
Fix CLI comment
blaginin Jul 7, 2025
481641a
Load cache for `linux-datafusion-common-features`
blaginin Jul 7, 2025
30986a4
Setup more caches
blaginin Jul 7, 2025
6488f47
Cache for clippy
blaginin Jul 7, 2025
53d54ed
comment clippy save
blaginin Jul 7, 2025
1a87552
Remove caches that aren't working
blaginin Jul 10, 2025
5000ae5
Merge branch 'main' into ci-cache
blaginin Jul 10, 2025
12c1a18
Save if
blaginin Jul 10, 2025
4da7d1c
Save if
blaginin Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/actions/setup-macos-aarch64-builder/action.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ runs:
rustup component add rustfmt
- name: Setup rust cache
uses: Swatinem/rust-cache@v2
Copy link
Copy Markdown
Contributor

@alamb alamb Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that rust-cache caches the Rust binaries too

https://github.com/Swatinem/rust-cache?tab=readme-ov-file#cache-details

Copy link
Copy Markdown
Contributor

@alamb alamb Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, something I noticed was maybe we should pin to an actual hash rather than a tag that is updated

https://github.com/Swatinem/rust-cache/tags

I think security best practice would be to pin to Swatinem/rust-cache@98c8021 for example rather than https://github.com/Swatinem/rust-cache/releases/tag/v2

However, this wasn't introduced in this PR so I don't think it needs to be fixed here

with:
save-if: ${{ github.ref_name == 'main' }}
Copy link
Copy Markdown
Contributor

@alamb alamb Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also have the shared-key: amd ?

Copy link
Copy Markdown
Member Author

@blaginin blaginin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a macos run, so I don't this we should use shared key with other runs (which are on linux)

If shared key isn't set, job-based one is used: https://github.com/Swatinem/rust-cache/blob/7e1e2d0a10862b34e5df481373b2b0f295d1a2ef/action.yml#L10

Copy link
Copy Markdown
Member Author

@blaginin blaginin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So should be good as is!

This line is added because we now cache CI runs on any branch, which takes up all our free CI space 😱 (which also made it hard to test during development)

- name: Configure rust runtime env
uses: ./.github/actions/setup-rust-runtime
42 changes: 39 additions & 3 deletions .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
shared-key: "amd-ci-check" # this job uses it's own cache becase check has a separate cache and we need it to be fast as it blocks other jobs
save-if: ${{ github.ref_name == 'main' }}
- name: Prepare cargo build
run: |
# Adding `--locked` here to assert that the `Cargo.lock` file is up to
Expand Down Expand Up @@ -99,6 +104,11 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
save-if: false # set in linux-test
shared-key: "amd-ci"
- name: Check datafusion-substrait (default features)
run: cargo check --profile ci --all-targets -p datafusion-substrait
#
Expand Down Expand Up @@ -162,6 +172,11 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
save-if: false # set in linux-test
shared-key: "amd-ci"
- name: Check datafusion (default features)
run: cargo check --profile ci --all-targets -p datafusion
#
Expand Down Expand Up @@ -247,15 +262,22 @@ jobs:
name: cargo test (amd64)
needs: linux-build-lib
runs-on: ubuntu-latest
container:
image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
submodules: true
fetch-depth: 1
- name: Setup Rust toolchain
run: rustup toolchain install stable
- name: Install Protobuf Compiler
run: sudo apt-get install -y protobuf-compiler
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
save-if: ${{ github.ref_name == 'main' }}
shared-key: "amd-ci"
- name: Run tests (excluding doctests and datafusion-cli)
env:
RUST_BACKTRACE: 1
Expand All @@ -279,6 +301,10 @@ jobs:
name: cargo test datafusion-cli (amd64)
needs: linux-build-lib
runs-on: ubuntu-latest
# should be uncommented once https://github.com/apache/datafusion/pull/16644 is merged
# and cache should be added
# container:
# image: amd64/rust
steps:
- uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -329,6 +355,11 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
save-if: ${{ github.ref_name == 'main' }}
shared-key: "amd-ci-linux-test-example"
- name: Run examples
run: |
# test datafusion-sql examples
Expand Down Expand Up @@ -655,6 +686,11 @@ jobs:
rust-version: stable
- name: Install Clippy
run: rustup component add clippy
- name: Rust Dependency Cache
uses: Swatinem/rust-cache@v2
with:
save-if: ${{ github.ref_name == 'main' }}
shared-key: "amd-ci-clippy"
- name: Run clippy
run: ci/scripts/rust_clippy.sh

Expand Down
Loading