ci: Disable cache save for pull requests in GitHub Actions #28292

hebasto · 2023-08-18T06:24:23Z

This PR disable cache save for pull requests in GitHub Actions.

Otherwise, multiple pull requests fill GitHub Actions cache quota shortly.

See a discussion here.

NOTE for the maintainers with "owner" permissions.

This PR needs the actions/cache/restore@* and actions/cache/save@* acrions to be explicitly allowed in the repository's Actions permissions.

DrahtBot · 2023-08-18T06:24:25Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	MarcoFalke

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko

concept ACK

.github/workflows/ci.yml

maflcko · 2023-08-18T06:32:20Z

.github/workflows/ci.yml

+          key: ${{ github.job }}-ccache-${{ github.run_id }}
+          restore-keys: ${{ github.job }}-ccache-


why does this specify both keys?

While we are interested in the restore-keys input only, the key input is required, thus, it cannot be avoided.

ok, maybe use the same value for both, or is there a reason to use different values?

... is there a reason to use different values?

Yes, it is. github.run_id is unique for every run. That is why the restore-keys input contains prefixes that are expected to match. The latest matched cache will be restored.

maflcko · 2023-08-18T06:34:51Z

.github/workflows/ci.yml

+
+      - name: Save Ccache cache
+        uses: actions/cache/save@v3
+        if: github.event_name != 'pull_request'


Maybe also disable it for branches other than master, to make sure that storage is constant, even if there was an intermittent push to a backport branch?

ok, nvm. Looks like caches are immutable anyway, so this can't be achieved, I guess.

Maybe it is still better to only have caches for a single branch, as they can't be re-used for other branches anyway?

In the main repository, caches for backport branches will be evicted after 7 days of non-using them.

However, the current logic seems useful for fork repos, no?

I was thinking that (assuming a cache size per push of $n GB, and a total limit of 10GB), 10/n pushes to a backport branch will evict the main cache. For example, for n=5, 2 pushes are enough.

Though, it is only a cache, so I guess it doesn't matter.

On a contributor's topic branch in their personal repo, the workflow will be triggered by a push event. So the updated push will reuse the cache.

Can anyone with a fork clobber our cache?

No.

Can someone elaborate on why branches in forks of this repository are something we need to consider when deciding on our caching stratergy for this repository?

I use CI in my personal repo before pushing my branch into the pull request. And I want to use CI with some efficiency.

On a contributor's topic branch in their personal repo, the workflow will be triggered by a push event. So the updated push will reuse the cache.

No? I think it will use the latest cache, which may be from a different branch, because the cache key isn't included the branch name, so I don't think this will work.

Edit: Maybe it does: https://github.com/actions/cache/blob/main/tips-and-workarounds.md#force-deletion-of-caches-overriding-default-cache-eviction-policy

On a contributor's topic branch in their personal repo, the workflow will be triggered by a push event. So the updated push will reuse the cache.

No? I think it will use the latest cache, which may be from a different branch, because the cache key isn't included the branch name, so I don't think this will work.

https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#example-using-multiple-restore-keys: a feature branch is considered before the default branch.

maflcko

lgtm ACK

.github/workflows/ci.yml

maflcko · 2023-08-18T07:18:30Z

I guess as an alternative, the ghcr can be used to store a ccache? This would require a bit more code on our side, and maybe a few more permissions to be enabled? Though, if the write-permissions are limited to the master branch, maybe that is fine?

hebasto · 2023-08-18T07:26:47Z

I guess as an alternative, the ghcr can be used to store a ccache? This would require a bit more code on our side, and maybe a few more permissions to be enabled?

For now, only two jobs, namely native macOS and Windows, are planned to run on GitHub Actions. It should work fine at this scale, no?

Though, if the write-permissions are limited to the master branch, maybe that is fine?

I'm not sure about how to make "the write-permissions are limited to the master branch". I'll do my research though.

hebasto · 2023-08-18T08:57:23Z

the ghcr can be used to store a ccache?

Anyway, it won't work with non-Linux runners out-of-the-box.

DrahtBot · 2023-08-18T15:03:49Z

Looks like CI is red, but it is not shown here?

https://github.com/bitcoin/bitcoin/actions/runs/5899744715

hebasto · 2023-08-18T15:08:36Z

Looks like CI is red, but it is not shown here?

bitcoin/bitcoin/actions/runs/5899744715

@achow101 @fanquake

I'm kindly asking you to update Actions permissions according to this PR description.

fanquake · 2023-08-21T10:10:50Z

I'm kindly asking you to update Actions permissions according to this PR description.

Should be available now.

Otherwise, multiple pull requests fill GitHub Actions cache quota shortly.

maflcko · 2023-08-21T14:12:47Z

lgtm ACK 241d6ca

…ub Actions 241d6ca ci: Disable cache save for pull requests in GitHub Actions (Hennadii Stepanov) Pull request description: This PR disable cache save for pull requests in GitHub Actions. Otherwise, multiple pull requests fill GitHub Actions cache quota shortly. See a discussion [here](bitcoin#28187 (comment)). --- **NOTE** for the maintainers with "owner" permissions. This PR needs the `actions/cache/restore@*` and `actions/cache/save@*` acrions to be explicitly allowed in the repository's Actions permissions. ACKs for top commit: MarcoFalke: lgtm ACK 241d6ca Tree-SHA512: a7786c7ec99bfa6991bf6ae08fd7ed3546e8c5d083a1b2bae7638f6f31e77fdf2cf4fc69d85834faf87f98db1f4a82026ce1dc5fc1bc6650e8bf1c09bf7e90f5

DrahtBot added the Tests label Aug 18, 2023

maflcko reviewed Aug 18, 2023

View reviewed changes

maflcko approved these changes Aug 18, 2023

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

hebasto force-pushed the 230818-gha-ccache branch from f1bd112 to d70fabd Compare August 18, 2023 06:56

DrahtBot added the CI failed label Aug 18, 2023

DrahtBot removed the CI failed label Aug 18, 2023

hebasto mentioned this pull request Aug 19, 2023

ci: Run Windows native task on GitHub Actions #28173

Merged

hebasto force-pushed the 230818-gha-ccache branch from d70fabd to a0f9b09 Compare August 21, 2023 10:14

ci: Disable cache save for pull requests in GitHub Actions

241d6ca

Otherwise, multiple pull requests fill GitHub Actions cache quota shortly.

hebasto force-pushed the 230818-gha-ccache branch from a0f9b09 to 241d6ca Compare August 21, 2023 10:26

hebasto requested a review from maflcko August 21, 2023 13:45

DrahtBot removed the request for review from maflcko August 21, 2023 14:12

fanquake merged commit ded6873 into bitcoin:master Aug 21, 2023

hebasto deleted the 230818-gha-ccache branch August 21, 2023 16:10

maflcko mentioned this pull request Jun 19, 2024

ci: Move more tasks to GHA? #30304

Closed

bitcoin locked and limited conversation to collaborators Aug 20, 2024

		key: ${{ github.job }}-ccache-${{ github.run_id }}
		restore-keys: ${{ github.job }}-ccache-

ci: Disable cache save for pull requests in GitHub Actions #28292

ci: Disable cache save for pull requests in GitHub Actions #28292

Uh oh!

Conversation

hebasto commented Aug 18, 2023

Uh oh!

DrahtBot commented Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Aug 18, 2023

Uh oh!

hebasto commented Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 18, 2023

Uh oh!

DrahtBot commented Aug 18, 2023

Uh oh!

hebasto commented Aug 18, 2023

Uh oh!

fanquake commented Aug 21, 2023

Uh oh!

maflcko commented Aug 21, 2023

Uh oh!

Uh oh!

DrahtBot commented Aug 18, 2023 •

edited

Loading

hebasto Aug 18, 2023 •

edited

Loading

hebasto Aug 18, 2023 •

edited

Loading

maflcko Aug 18, 2023 •

edited

Loading

hebasto Aug 18, 2023 •

edited

Loading

hebasto commented Aug 18, 2023 •

edited

Loading