feat: Add support for in-memory caching #1028
Conversation
|
Well, the test failure shows that we indeed need to have immutable caches before we start to do anything with in-memory caching :-( |
Swatinem
force-pushed
the
ref/memory-cache
branch
3 times, most recently
from
a7086be
to
44659e7
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1028 +/- ##
==========================================
+ Coverage 74.96% 75.15% +0.18%
==========================================
Files 83 83
Lines 11912 11981 +69
==========================================
+ Hits 8930 9004 +74
+ Misses 2982 2977 -5 |
Fixes #994 by adding the infrastructure and defaults for in-memory caching. This implements weighing of cache items (based on a bunch of `size_of`s), and per-item TTL based on the `ExpirationTime`. Each cache defaults to ~100k in-memory size, which should be roughly ~1k items. Except for `object_meta` which defaults to ~100M in-memory and is very hot, and `cficaches` which defaults to ~400M and are expensive to parse.
Swatinem
force-pushed
the
ref/memory-cache
branch
from
e258391
to
e15351f
Compare
| // FIXME: `object_meta` caches treat download errors as `malformed` | ||
| config.caches.derived.retry_malformed_after = Some(Duration::ZERO); |
There was a problem hiding this comment.
I believe this has been broken ever since https://github.com/getsentry/symbolicator/pull/516/files#diff-2f856962a69fa5bec61ba23e49b676ce0d95db30e21d49f938da24302cb8b3b4R343-R349
object_meta are a Derived cache, so server errors were only being retried after the "malformed" timeout since basically forever.
|
|
||
| /// Capacity (in bytes) for the in-memory `object_meta` Cache. | ||
| /// | ||
| /// Defaults to `100 MiB (= 104_857_600)`. | ||
| pub object_meta_capacity: u64, | ||
|
|
||
| /// Capacity (in bytes) for the in-memory `cficaches` Cache. | ||
| /// | ||
| /// Defaults to `400 MiB (= 419_430_400)`. | ||
| pub cficaches_capacity: u64, |
There was a problem hiding this comment.
To be honest I'm not too fond of the entire InMemoryCacheConfig struct. For these options in particular I think it would be nicer if they lived directly in their respective cache configurations (i.e. add an in_memory_capacity field to DownloadedCacheConfig and DerivedCacheConfig), but I understand that would make setting these defaults impossible.
There was a problem hiding this comment.
I would rather like to get rid of the difference between DownloadedCacheConfig and DerivedCacheConfig altogether. The comment I posted above shows that we have bugs related to the retry_after timings. And the only difference between the two is the time-to-idle, which we could express as their own explicit config settings.
There was a problem hiding this comment.
Yeah actually that would be even better.
Co-authored-by: Sebastian Zivota <[email protected]>
Fixes #994 by adding the infrastructure and defaults for in-memory caching.
This implements weighing of cache items (based on a bunch of
size_ofs),and per-item TTL based on the
ExpirationTime.Each cache defaults to ~100k in-memory size, which should be roughly ~1k items.
Except for
object_metawhich defaults to ~100M in-memory and is very hot,and
cficacheswhich defaults to ~400M and are expensive to parse, with variable per-item weight.