Skip to content

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

Merged
encukou merged 1 commit into
python:mainfrom
encukou:gha-ccache
Jan 10, 2024
Merged

GH-113858: GitHub Actions config: Only save ccache on pushes#113859
encukou merged 1 commit into
python:mainfrom
encukou:gha-ccache

Conversation

@encukou

@encukouencukou commented Jan 9, 2024

Copy link
Copy Markdown
Member

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.

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

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

zware commented Jan 9, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
MemberAuthor

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

zware commented Jan 9, 2024

Copy link
Copy Markdown
Member

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

@gpshead

Copy link
Copy Markdown
Member

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. :)

@encukouencukou removed needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Jan 10, 2024
@encukou

Copy link
Copy Markdown
MemberAuthor

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:mainJan 10, 2024
@encukou
encukou deleted the gha-ccache branch January 10, 2024 08:49
encukou added a commit that referenced this pull request Jan 16, 2024
…) (GH-114082)
This backports 3 PRs:
- #113859
Only save ccache on pushes
- #113945
Cut down ccache size
- Only save the ccache in the main reusable builds, not on builds that
don't use special build options:
- Generated files check
- OpenSSL tests
- Hypothesis tests
- Halve the max cache size, to 200M
- #114113
Fixup for the above
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
jessecomeau87 pushed a commit to jessecomeau87/Python that referenced this pull request May 20, 2024
This backports 3 PRs:
- python/cpython#113859
Only save ccache on pushes
- python/cpython#113945
Cut down ccache size
- Only save the ccache in the main reusable builds, not on builds that
don't use special build options:
- Generated files check
- OpenSSL tests
- Hypothesis tests
- Halve the max cache size, to 200M
- python/cpython#114113
Fixup for the above
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@encukou@zware@gpshead@hugovk