CI: Add workflow to cleanup PR caches when closed #104077
Conversation
This is pretty much copied from the GitHub Actions documentation: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows#force-deleting-cache-entries
Repiteo
left a comment
Repiteo
left a comment
There was a problem hiding this comment.
You'd think they'd automate this, because PR caches cannot be reused by anything at that point. Oh well!
AThousandShips
left a comment
AThousandShips
left a comment
There was a problem hiding this comment.
Hard to confirm on its own but I trust the GitHub docs on this
| BRANCH: refs/pull/${{ github.event.pull_request.number }}/merge | ||
| run: | | ||
| echo "Fetching list of cache key" | ||
| cache_keys_for_pr=$(gh cache list --ref $BRANCH --limit 100 --json id --jq '.[].id') |
There was a problem hiding this comment.
Note that this deletes only up to 100 caches (that's what the docs example does).
After #104076 we should only have a single cache key pattern per PR, so maybe around 20 caches totally from all our jobs, so it should be fine.
Currently this might not even include all caches in a PR where e.g. a commit was amended 10 times, which would have around 200 unique caches. The rest would be culled after 7 days.
There was a problem hiding this comment.
So since #104076 isn't going to work, maybe we need to reconsider how many caches we want to query here, as we might have multiple hundreds in a lot of PRs.
There was a problem hiding this comment.
100 caches would be a bit over 5 runs of a PR, that's not an unreasonable case, 200 would be pretty sufficient I'd say, that's 10 recent runs
Note also that a significant portion of PRs are merged when they have no caches around, or at most one set likely, the general retention for caches with the pressure we have is about a day, essentially always no older than two days (never seen a cache on the repo that old AFAIK)
So only cases where a PR is amended repeatedly shortly before (within 24 hours) being merged would we need to worry about that I'd say (I occasionally go through and check for old caches that are for merged PRs and there often aren't any/many)
There was a problem hiding this comment.
I checked quickly some recent PRs that had multiple commits today and they all have only 19 caches. So it sounds like previous caches already get dropped somehow, maybe because we're already at such a saturated level that they quickly get garbage collected.
We can reassess after we've reduced our cache congestion further to see if it needs to be increased.
There was a problem hiding this comment.
PRs that were merged recently?
There was a problem hiding this comment.
No some that are still open and had multiple commits today, e.g. the Traits PR.
akien-mga
added
the
cherrypick:3.x
|
The one area where this would be annoying would be where PRs are closed due to other reasons, for example a rebase error, but that seems rare enough not to be an issue |
|
Seem to unfortunately not work, see: Probably because the token doesn't have any write permissions: |
|
That's weird, the config on this repo is:
And I don't see us limiting |
|
This example uses a different syntax, might be required to make it work: |
akien-mga
removed
the
cherrypick:3.x
3 participants
This is pretty much copied from the GitHub Actions documentation: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows#force-deleting-cache-entries
To test this properly, it needs to be merge so that it's active, and then we should monitor what happens when merging PRs. I've tested this in a non-Godot repo and it works fine there, but the cache setup is slightly different.
Related to #104076 but can be merged independently.