-
Notifications
You must be signed in to change notification settings - Fork 17
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
Invalidate the cache when the stack changes #21
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I've tested all four scenarios: Clean cache build using this branch
Warm cache build showing cache reuse
Warm cache build showing cache invalidation on stack change
Migration of existing cache written by older version of this buildpack
|
edmorley
commented
Jun 19, 2024
edmorley
force-pushed
the
edmorley/cache-invalidation-stack
branch
from
June 19, 2024 14:26
54e3e3c
to
52f05de
Compare
mars
approved these changes
Jun 20, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful, this will avoid so many issues as customers migrate to newer stacks!
Previously the buildpack did not invalidate the cache when the stack changed, which causes the errors seen in #19. This will be particularly an issue when Heroku-24 is GAed and Heroku-20 deprecated, since there will be a number of apps changing stack for the first time after switching to this buildpack. Now, the stack version is stored in the cache and if it differs from the current stack version (or the version file is missing since the cache is from an older version of this buildpack), the cache will be purged. This implementation is an improved version of what's implemented for the generic APT buildpack (I'll be backporting those improvements over to that buildpack shortly): https://github.com/heroku/heroku-buildpack-apt/blob/4eb4b35d35d0178e5cef73d6998a26ba2c9bbf17/bin/compile#L42-L86 Longer term we should probably re-evaluate whether we need the cache at all, since from my testing locally it offered very little benefit, and if anything causes a number of issues (particularly when several APT-using buildpacks are set on an app at the same time, given the shared Debian archives directory and install strategy). Fixes #19.
edmorley
force-pushed
the
edmorley/cache-invalidation-stack
branch
from
June 21, 2024 10:11
52f05de
to
2736ead
Compare
Released in:
|
edmorley
added a commit
to heroku/heroku-buildpack-apt
that referenced
this pull request
Jun 24, 2024
Now: - If the stack version file is missing, we don't assume the cached files belong to the current stack (since that could cause breakage), and instead invalidate the cache. (The cache actually offers little benefit in practice for this buildpack, so invalidating is cheap.) - The whole `${CACHE_DIR}/apt` directory is removed rather than only the `${CACHE_DIR}/apt/cache` directory. This ensures the APT indexes and other files from old stack versions are cleaned up too. - Some build output logs have been adjusted to be more accurate. - A test has been added for cache re-use, since it wasn't previously tested. These papercuts were noticed whilst working on: heroku/heroku-buildpack-chrome-for-testing#21
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the buildpack did not invalidate the cache when the stack changed, which causes the errors seen in #19. This will be particularly an issue when Heroku-24 is GAed and Heroku-20 deprecated, since there will be a number of apps changing stack for the first time after switching to this buildpack.
Now, the stack version is stored in the cache and if it differs from the current stack version (or the version file is missing since the cache is from an older version of this buildpack), the cache will be purged.
This implementation is an improved version of what's implemented for the generic APT buildpack (I'll be backporting those improvements over to that buildpack shortly):
https://github.com/heroku/heroku-buildpack-apt/blob/4eb4b35d35d0178e5cef73d6998a26ba2c9bbf17/bin/compile#L42-L86
Longer term we should probably re-evaluate whether we need the cache at all, since from my testing locally it offered very little benefit, and if anything causes a number of issues (particularly when several APT-using buildpacks are set on an app at the same time, given the shared Debian archives directory and install strategy).
Fixes #19.
GUS-W-16045454.