Skip to content
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.

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

GH-113858: GitHub Actions config: Only save ccache on pushes #113859

Merged
merged 1 commit into from Jan 10, 2024

Conversation

encukou
Copy link
Member

@encukou encukou commented Jan 9, 2024

This should save a new cache only on commits that are pushed to CPython branches, not on in-progress PRs.

AFAIK the action definitions won't apply until they're merged. I've tested this in my branch (PR with this change, dummy PR on top). Some of the builds there failed due to a GitHub outage.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looking at one of this PR's jobs, it restored the cache correctly:

Run hendrikmuhs/ccache-action@v1.2
  with:
    save: false
    restore: true
    max-size: 500M
    verbose: 0
    variant: ccache
    append-timestamp: true
    create-symlink: false
  env:
    OPENSSL_VER: 3.0.11
    PYTHONSTRICTEXTENSIONBUILD: 1
    MULTISSL_DIR: /home/runner/work/cpython/cpython/multissl
    OPENSSL_DIR: /home/runner/work/cpython/cpython/multissl/openssl/3.0.11
    LD_LIBRARY_PATH: /home/runner/work/cpython/cpython/multissl/openssl/3.0.11/lib
    PATH: /usr/lib/ccache:/snap/bin:/home/runner/.local/bin:/opt/pipx_bin:/home/runner/.cargo/bin:/home/runner/.config/composer/vendor/bin:/usr/local/.ghcup/bin:/home/runner/.dotnet/tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin
Restore cache
  Received [2](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:2)81018[3](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:3)68 of [4](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:4)240[5](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:5)2057 ([6](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:6)6.3%), 26[7](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:7).7 MBs/sec
  Cache Size: ~404 MB (424052057 B)
  /usr/bin/tar -xf /home/runner/work/_temp/cab072ac-b[8](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:8)00-4c22-8c18-37df3b781a[9](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:9)9/cache.tzst -P -C /home/runner/work/cpython/cpython --use-compress-program unzstd
  Received 424052057 of 424052057 ([10](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:10)0.0%), 202.0 MBs/sec
  Cache restored successfully
  Restored from cache key "ccache-2024-01-09T10:51:21.[16](https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859#step:9:16)8Z".
Configure ccache, linux

And correctly didn't save a new cache:

Post job cleanup.
ccache stats
Not saving cache because 'save' is set to 'false'.

https://github.com/python/cpython/actions/runs/7462201364/job/20303997133?pr=113859

@zware
Copy link
Member

zware commented Jan 9, 2024

IIUC though, we are going to want to adjust the cache key(s) somewhat to avoid keeping a bunch of timestamped caches anyway. Setting append-timestamp: false might be enough, but we may also want to add a per-branch key.

@encukou
Copy link
Member Author

encukou commented Jan 9, 2024

The cache needs to be reasonably fresh to be useful, though.
GitHub doesn't allow updating caches, so we need to generate new ones occasionally. And I don't think we want to write tooling to evict old caches, so (if we don't want to remove ccache altogether) we will keep 10GB of old caches around.
We just need to generate them more slowly, so they don't crowd the other caches.

@zware
Copy link
Member

zware commented Jan 9, 2024

GH won't let you overwrite a cache entry? That sounds...broken :)

@gpshead
Copy link
Member

gpshead commented Jan 9, 2024

I think a per branch key is a good idea. realistically only two branches see regular changes. if this means CI is slower when doing a security backport on those branches for the initial run before they generate an update to their long since timed out and deleted cache, that isn't a problem. We want the fastest CI and best ccache behavior in main and the current release branch.

as for not being able to overwrite a specific entry, i can understand the GH infrastructure design reasons behind that. live with it. :)

@encukou encukou removed needs backport to 3.11 bug and security fixes needs backport to 3.12 bug and security fixes labels Jan 10, 2024
@encukou
Copy link
Member Author

encukou commented Jan 10, 2024

realistically only two branches see regular changes.

But that also means we don't really need to care about occasional cache generated from older branches :)


Well, I'll merge and see how this plays out; if it's fine I'll do the backports.

@encukou encukou merged commit 5d384b0 into python:main Jan 10, 2024
38 checks passed
@encukou encukou deleted the gha-ccache branch January 10, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants