Timed based addon cache #1389
Merged
Timed based addon cache #1389
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
nirs
force-pushed
the
drenv-cache
branch
2 times, most recently
from
260373d to
ed0c730
Compare
nirs
force-pushed
the
drenv-cache
branch
4 times, most recently
from
9049863 to
4def0e9
Compare
nirs
force-pushed
the
drenv-cache
branch
3 times, most recently
from
74d9a06 to
31cc35a
Compare
nirs
force-pushed
the
drenv-cache
branch
2 times, most recently
from
f6aa183 to
a05f35a
Compare
And remove the timing of the operation, deleting a directory is fast local operation. Signed-off-by: Nir Soffer <[email protected]>
Called to warn about a missing addon. Signed-off-by: Nir Soffer <[email protected]>
Previously we had to do:
path = cache.patch(key)
cache.fetch(kustomziation_dir, key)
Now we do both operations in one call:
path = cache.fetch(kustomziation_dir, key)
Thanks: Raghavendra Talur <[email protected]>
Signed-off-by: Nir Soffer <[email protected]>
Extract the a _fetch() helper to be used by new public APIs. Signed-off-by: Nir Soffer <[email protected]>
Preparing for renaming "fetch" to "cache" - the cache() function will shadow the cache module. We can rename the imports or the function to avoid this. Renaming the import is bad since it makes it harder to understand the code. Renaming the functions is good since it helps to understand what is a command function. Signed-off-by: Nir Soffer <[email protected]>
Previously to update the cache you had to clear the cache and fetch. This leaves a window where a build may use uncached value and fail. A better way is to refresh the cache periodically *without* clearing the cache, ensuring that builds will always use cached values. To support time based refresh, we fetch data only if the cached item is too old or missing. Upon successful fetch, the item age is set to zero, so the next access to the cache will skip the fetch. We use 2 threshold for refreshing the cache: - REFRESH_THRSHOLD: 12 hours - FETCH_THRESHOLD: 48 hours This allows refreshing the cache once per day (e.g. from a cron job). On errors, we can rerun the refresh until the refresh succeeds. Fresh items are skipped during the refresh. If refreshing the cache failed, jobs will trigger a refresh after 48 hours. The longer threshold ensures that this will never happen unless the refresh job is broken. The `fetch` command was renamed to `cache`. This describe better the purpose of the command. The hook was update as well to match the command name. The `drenv.cache` module provides now 2 operations: - cache.refresh() - rebuild the cache if older than cache.REFRESH_THRESHOLD (12 hours). Will is used in the `cache` hook. - cache.get() - rebuild the cache if older than cache.FETCH_THRESHOLD (48 hours) and return the path to the cached value. Will is used in `start` hooks to get a cached value. Fixes: RamenDR#1435 Signed-off-by: Nir Soffer <[email protected]>
Add script and crontab sample: - `scripts/refresh-cache`: can be run from a cron job to refresh the cache. - `scripts/refresh-cache.crontab`: sample user crontab for refreshing the cache daily. Signed-off-by: Nir Soffer <[email protected]>
ShyamsundarR
approved these changes
Jun 20, 2024
nirs
added a commit
to nirs/ramen
that referenced
this pull request
Jun 20, 2024
We change the cache API in RamenDR#1389 but it was merge after the argocd was added. - Use cache.get() instead of removed cache.fetch() - Change current directory for start hook was missing in RamenDR#1421, not sure how the cache worked without it. Signed-off-by: Nir Soffer <[email protected]>
ShyamsundarR
pushed a commit
that referenced
this pull request
Jun 20, 2024
We change the cache API in #1389 but it was merge after the argocd was added. - Use cache.get() instead of removed cache.fetch() - Change current directory for start hook was missing in #1421, not sure how the cache worked without it. Signed-off-by: Nir Soffer <[email protected]>
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.
Improve the caching logic to refresh the cache automatically base on cached item age. This ensures that we always use fresh cache (< 2 days), so new changes in addons resources are used automatically and transparently for developers and CI.
The time based logic also avoids the issue of having to clear the cache to trigger a refresh of the cache, eliminating refreshes during builds. We can run now
drenv cache envfilefrom a daily cron job, to update the cached resources every day.The time based logic also allows retries on
drenv cacheerrors. We can run a simple retry loop runningdrenv cacheevery 5 minutes until it succeeds. Cached items newer than 12 hours are skipped automatically.Fixes #1435