Disk cache garbage collection

[calebzulawski]

My initial implementation of garbage collection for #18. It seems to work but it could use more testing.

[calebzulawski]

I am seeing some odd errors, like:

Failed to save: Unable to reserve cache with key setup-bazel-1-darwin-disk-MY_CACHE_KEY-e211a56abda1473b71bb15b46a58c96b588692bd155524ecbb2bdb23e5547203, another job may be creating this cache.

I don't actually have conflicting jobs (as far as I can tell), so not sure what's going on.

[calebzulawski]

It turns out, according to actions/toolkit#658, you can't actually update an existing key. The only option is creating a new key, so I add a unique identifier to the end, and restore it via a prefix match.

[p0deje]

@calebzulawski please ping me for review/testing once you're happy with implementation

[calebzulawski]

@p0deje sorry for all the churn, I can clean up the history if you'd like, but I think this is good to go.

A quick summary of changes:

• Garbage collect disk and repository caches
  • These caches are keyed by the hash of their contents rather than BUILD files etc
  • Contents are sorted by mtime, removing the oldest files until the cache is under the size maximum
  • A future change could also add an age maximum
• Add cache-prefix argument that affects all caches (this replaces non-bool values for disk-cache, too)

[p0deje]

I suppose we can drop this input and move it to cache-prefix instead.

[calebzulawski]

What's the purpose of this input? If the prefix is user-configurable, is it necessary at all? Should the default still have 1 in it?

[p0deje]

Let's add a core.warning when repository-cache is not a boolean. This would indicate that an old interface for using repository-cache with a path to a file is being used, and we can add a link to the release page with upgrade instructions (https://github.com/bazel-contrib/setup-bazel/releases/tag/0.15.0)

[calebzulawski]

I ended up adding a core.error, because ultimately the action will fail

[p0deje]

Let's add a core.warning when disk-cache is not a boolean. This would indicate that an old interface for using disk-cache with a cache name is being used, and we can add a link to the release page with upgrade instructions (https://github.com/bazel-contrib/setup-bazel/releases/tag/0.15.0)

[p0deje]

Let's clarify it's only used for disk/repository cache, not bazlisk/external.

[calebzulawski]

The way I implemented it, all of the caches use this prefix, including bazelisk and external. This resolves #73

[p0deje]

After trying to use it in Selenium (SeleniumHQ/selenium#15427), I believe we might need to separate inputs for prefixes for repository and disk caches. The main reason is that I see lots of similar repository caches for matrix jobs, and the Selenium project would benefit from having a single repository cache (GC'ed) while keeping disk caches separate for each job.

[calebzulawski]

We could do that, but on cache misses you will continue to see the kinds of errors in #73, as each job tries to write the same cache. Alternatively, if they aren't actually completely identical caches (e.g. the jobs have slightly different use of dependencies), whichever job completes last will win out for the next run and might end up missing some of the dependencies. You could even end up with a dependency repeatedly purged because the job that completes last doesn't use it and doesn't update its mtime.

What about adding an input for additional restore prefixes, so those jobs could share caches rather than have a cache miss?

[p0deje]

@calebzulawski Sorry for the delay, it's been a crazy couple of months and I couldn't keep up with all the OSS projects I have on me. I'll go through our conversation and PR again to see where we left off.

[calebzulawski]

No worries, thanks!