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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: clear old caches to avoid master branch cache trashing #14174

Merged
merged 1 commit into from
May 19, 2024

Conversation

kasper93
Copy link
Contributor

@kasper93 kasper93 commented May 19, 2024

No description provided.

Copy link

github-actions bot commented May 19, 2024

Download the artifacts for this pull request:

Windows
macOS

@kasper93 kasper93 force-pushed the ci_cache branch 4 times, most recently from 8515eb8 to 9fde79c Compare May 19, 2024 17:04
@kasper93 kasper93 requested review from sfan5 and Dudemanguy May 19, 2024 17:05
@kasper93
Copy link
Contributor Author

kasper93 commented May 19, 2024

I tried to keep things simple and not overcomplicate with some artifacts passing PR number or other "tricks" that initially made sense, but after all doing validation on all cache items should be good.

EDIT: Note that this will run only after merged to master. PR has no ability to run this code for security reasons, and that a good thing. (little bit annoying to test though :))

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

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

Didn't look too deeply into github actions details, but seems fine to me.

@kasper93
Copy link
Contributor Author

Ok, let's see how it works in practice.

GitHub cache action doesn't allow updating cache with the same key. We
workaround this by saving the cache with a unique key each time (added
timestamp). This works fine, but since there is a limit on cumulative
storage size for all caches, it can force the master branch cache to be
evicted if a lot of PRs are updated. Cache is evicted with LRU policy,
so as long as master branch cache is used it should stay alive, but it
can happen that only PR specifc caches were only used. As a reminder,
PRs can access the master cache, but they are isolated from each other.
Because of this, it is important to keep the master cache, which is
available to all, alive longer.

The solution is to remove all old caches per branch. This is done in a
separate workflow that validates all cache items and removes ones that
would never be used anyway. If PR is closed all caches per branch are
removed. In other cases most recently used one is preserved.

It is done in a separate workflow to limit cache manipulation access.
GitHub workflows triggered by pull_request event are run in the context
of the fork and does not have access to our token, which is good thing.
Also it is quite awkward to get PR number which triggered build
workflow, so just do a full cleanup pass.
@kasper93 kasper93 merged commit c6b950a into mpv-player:master May 19, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants