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

Use restoreKeys with cache #239

Closed
wants to merge 1 commit into from

Conversation

dnephin
Copy link

@dnephin dnephin commented Jun 17, 2022

Description:

Without restoreKeys any change to go.sum results in a complete cache miss. By using restoreKeys a change in one dependency should still allow a cache to be restored, so that all the other dependencies benefit from the cache.

This changes matches the recommendation documented here:
https://github.com/actions/cache/blob/main/examples.md#go---modules

The prefix used with restoreKeys includes the platform and Go version, because only those caches are likely to provide anything useful.

Related issue:

Builds on the recently added cache option from #228.

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

To match the recommendation documented here:
https://github.com/actions/cache/blob/main/examples.md#go---modules

Without `restoreKeys` any change to `go.sum` results in a complete cache
miss. By using `restoreKeys` a change in one dependency should still
allow a cache to be restored, so that all the other dependencies are
retreived from the cache.
@dnephin dnephin requested a review from a team June 17, 2022 18:19
@IvanZosimov
Copy link
Contributor

Hi, @dnephin 👋 ! Actually, we deliberately decided not to use restore keys. The reason for such a decision is a so-called "dependencies snowball effect". Imagine that your dependencies are cached and restore keys are implemented as you suggest.
So now in cache you have: [old dependencies]. Then you decide to use new library and add it to your dependencies. If we run action after that we will get [old dependencies] as a cache and new dependencies will be downloaded by the GO. Then our action will save new cache entry and it will consume all dependencies from the cache folder: [ old dependencies + new dependencies ]. If we repeat that process several times our cache entry will become enormously large - GO doesn't delete unuseful dependencies automatically. It will lead to performance decreasing (Imagine that you have to download several GB's of cache). This is what we call "dependencies snowball effect".

@dnephin
Copy link
Author

dnephin commented Jun 22, 2022

Ah right, makes sense! The build I am working with benefits very little from the module cache. Module download time is only 10s. Most of the benefit comes from the build cache. Some dependencies call gcc, which can take much longer than the Go compiler. Build time is over 2 minutes without the cache.

Right now the cache will reset every time a dependency changes. With the changes in this PR the cache would only reset when the Go version changes. Given that Go releases a new version twice a year, and only supports the last 2 versions, I imagine most people are updating their Go version at least once a year.

One option to address the cache growth problem would be to have a schedule job run once a week to clear the cache with go clean -cache -modcache -testcache. I think that would allow the build to always run quickly when run for PRs, without the cache growing indefinitely. However that is more work to setup.

Do you think resetting the cache on version change is enough to mitigate the cache growth problem?

@IvanZosimov
Copy link
Contributor

Hi, @dnephin 👋 ! Thank you for the clarification, unfortunately, we can't predict whether it is enough to reset cache after some time period or not because it depends only on the current use case. In your case, I suggest using actions/cache action because the more precise tuning is available there.

By now I'm closing this PR, if you have any other questions or additions - feel free to ping us!

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