feat: Add metadata to cache entries #1617
Merged
Conversation
This file contains hidden or 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
tobias-wilfert
approved these changes
Feb 11, 2025
jjbayer
approved these changes
Feb 11, 2025
|
|
||
| /// A cache entry with optional metadata. | ||
| #[derive(Debug, Clone)] | ||
| pub struct MdCacheEntry<T> { |
Member
There was a problem hiding this comment.
nit: Md might confuse readers, should we call this CacheEntry and the original CacheEntry CacheContents or similar?
Member
There was a problem hiding this comment.
I had the exact same thought, CacheEntry -> CacheContents and MdCacheEntry -> CacheEntry.
|
|
||
| impl<T> MdCacheEntry<T> { | ||
| /// Create a cache entry without attached metadata. | ||
| pub(crate) fn without_md(contents: CacheEntry<T>) -> Self { |
Member
There was a problem hiding this comment.
nit
Suggested change
| pub(crate) fn without_md(contents: CacheEntry<T>) -> Self { | |
| pub(crate) fn without_metadata(contents: CacheEntry<T>) -> Self { |
Dav1dde
approved these changes
Feb 12, 2025
|
|
||
| /// A cache entry with optional metadata. | ||
| #[derive(Debug, Clone)] | ||
| pub struct MdCacheEntry<T> { |
Member
There was a problem hiding this comment.
I had the exact same thought, CacheEntry -> CacheContents and MdCacheEntry -> CacheEntry.
Contributor
Author
|
Thanks for the feedback, will rename. |
loewenheim
added a commit
that referenced
this pull request
Feb 18, 2025
This adds functionality to write and read the metadata structures introduced in #1617 to and from the filesystem. A cache file's metadata file has the same path, but with the extension `.metadata`. The read happens in check_expiry, which gives the cleanup job access to the metadata. All of the old logic is still there—if no metadata file exists, we use the mtime from the FS as before. The write happens in compute. It should be noted that currently metadata are not propagated through the shared cache; this is left as future work.
loewenheim
added a commit
that referenced
this pull request
Feb 19, 2025
This adds a setting retry_missing_after_public to downloaded and derived cache configs. The effect of this setting is to control the time after which negative (missing, failed download, &c.) cache entries from public sources. Whether a file comes from a public source is determined by reading its metadata, which we added in #1617 and started writing in #1618. The default value for this setting is 24h. This means that each Symbolicator only makes one request per day to public symbol sources for each missing debug file, as opposed to one per hour like before.
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.
This attaches an (optional) metadata struct to cache entries produced by
compute_memoized. For now, this structure contains two pieces of data:We may add more in the future.
We don't currently write or read the metadata at any point. When we create a
MdCacheEntrywe use thewithout_mdconstructor, and when we consume one, we always usecontents()orinto_contents()to get the data. Actually using the metadata is left for a future PR.ETA: After discussion, this now also renames the former
CacheEntrytoCacheContentsand uses the nameCacheEntryfor the new "cache contents + metadata" struct.