-
Notifications
You must be signed in to change notification settings - Fork 297
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
RPC cache system #1083
Draft
tchataigner
wants to merge
17
commits into
keep-starknet-strange:main
Choose a base branch
from
tchataigner:feature/rpc-cache
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
RPC cache system #1083
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
4740ea6
chore: keep-starknet-strange#947: document cache task propagation for…
tchataigner 30b320f
feat: keep-starknet-strange#947: base lru cache
tchataigner 3d5b705
fixup! feat: keep-starknet-strange#947: base lru cache
tchataigner b5721f1
chore: keep-starknet-strange#947: fix typo
tchataigner 91b16b6
feat: keep-starknet-strange#947: cache task & start integration
tchataigner 9bd3e7f
chore: keep-starknet-strange#947: fix linter warning
tchataigner 5558eb9
feat: keep-starknet-strange#947: added storage value for blocks
tchataigner aa20b9b
feat: keep-starknet-strange#947: using data cache
tchataigner c52c580
Merge branch 'main' into feature/rpc-cache
tchataigner 4859118
feat: keep-starknet-strange#947: remove cache size management by value
tchataigner 483c758
feat: keep-starknet-strange#947: cmd arg to manage cache allocated size
tchataigner 13bfd17
feat: keep-starknet-strange#947: merge main
tchataigner 3095801
chore: keep-starknet-strange#947: cancel package-lock updatge
tchataigner b63d1a8
chore: keep-starknet-strange#947: fix linter warning
tchataigner 6c2bc53
chore: keep-starknet-strange#947: fix taplo warning
tchataigner 8de43d9
Update crates/client/rpc/src/cache/lru_cache.rs
tchataigner 59b7c99
Update crates/client/rpc/src/cache/lru_cache.rs
tchataigner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
use scale_codec::Encode; | ||
use schnellru::{ByMemoryUsage, LruMap}; | ||
|
||
/// Structure used for caching over the RPC layer. It is heavily inspired from [frontier's | ||
/// implementation](https://github.com/paritytech/frontier/blob/194f6bb06152402ba44b340c3d401ae6e0670d96/client/rpc/src/eth/cache/mod.rs#L76). | ||
pub struct LRUCache<K, V> { | ||
/// Cache name. | ||
name: &'static str, | ||
/// Inner cache, with different variant from [`InnerCache`] | ||
inner: LruMap<K, V, ByMemoryUsage>, | ||
/// Metrics generated by our cache for prometheus. | ||
metrics: Option<LRUCacheByteLimitedMetrics>, | ||
} | ||
|
||
enum CacheAction { | ||
Hit, | ||
Miss, | ||
Insert, | ||
} | ||
|
||
impl<K: Eq + core::hash::Hash, V: Encode> LRUCache<K, V> { | ||
/// Instantiate a new cache with a given name and size limit. If a prometheus registry is passed, | ||
/// then we register our cache metrics. | ||
pub fn new( | ||
cache_name: &'static str, | ||
max_allocated_size: usize, | ||
prometheus_registry: Option<prometheus_endpoint::Registry>, | ||
) -> Self { | ||
// If a registry was provided, then register our cache metrics to it. | ||
let metrics = match prometheus_registry { | ||
Some(registry) => match LRUCacheByteLimitedMetrics::register(cache_name, ®istry) { | ||
Ok(metrics) => Some(metrics), | ||
Err(e) => { | ||
log::error!(target: cache_name, "Failed to register metrics: {:?}", e); | ||
None | ||
} | ||
}, | ||
None => None, | ||
}; | ||
// Return new empty cache. | ||
Self { name: cache_name, inner: LruMap::new(ByMemoryUsage::new(max_allocated_size)), metrics } | ||
} | ||
|
||
/// Try to retrieve a value at a given key from our cache. If metrics are active we update them | ||
/// accordingly. | ||
pub fn get(&mut self, k: &K) -> Option<&mut V> { | ||
let action = if self.inner.get(k).is_some() { CacheAction::Hit } else { CacheAction::Miss }; | ||
self.update_metrics(action); | ||
self.inner.get(k) | ||
} | ||
|
||
/// Try to insert a new value at a given key in our cache. Also updates our metrics to the | ||
/// current cache size. | ||
pub fn insert(&mut self, k: K, v: V) -> bool { | ||
// Try to insert the entry. | ||
if self.inner.insert(k, v) { | ||
self.update_metrics(CacheAction::Insert); | ||
true | ||
} else { | ||
log::warn!(target: self.name, "Failed to insert entry into cache"); | ||
false | ||
} | ||
} | ||
|
||
/// Utility function to handle metrics update. | ||
fn update_metrics(&self, action: CacheAction) { | ||
if let Some(metrics) = &self.metrics { | ||
match action { | ||
CacheAction::Hit => metrics.hits.inc(), | ||
CacheAction::Miss => metrics.miss.inc(), | ||
CacheAction::Insert => metrics.allocated_memory_size.set(self.inner.memory_usage() as u64), | ||
} | ||
} | ||
} | ||
} | ||
|
||
struct LRUCacheByteLimitedMetrics { | ||
hits: prometheus::IntCounter, | ||
miss: prometheus::IntCounter, | ||
allocated_memory_size: prometheus_endpoint::Gauge<prometheus_endpoint::U64>, | ||
} | ||
|
||
impl LRUCacheByteLimitedMetrics { | ||
pub(crate) fn register( | ||
cache_name: &'static str, | ||
registry: &prometheus_endpoint::Registry, | ||
) -> Result<Self, prometheus_endpoint::PrometheusError> { | ||
Ok(Self { | ||
hits: prometheus_endpoint::register( | ||
prometheus::IntCounter::new( | ||
format!("madara_starknet_{}_hits", cache_name), | ||
format!("Hits of starknet {} cache.", cache_name), | ||
)?, | ||
registry, | ||
)?, | ||
miss: prometheus_endpoint::register( | ||
prometheus::IntCounter::new( | ||
format!("madara_starknet_{}_miss", cache_name), | ||
format!("Misses of starknet {} cache.", cache_name), | ||
)?, | ||
registry, | ||
)?, | ||
allocated_memory_size: prometheus_endpoint::register( | ||
prometheus_endpoint::Gauge::new( | ||
format!("madara_starknet_{}_size", cache_name), | ||
format!("Allocated memory size of starknet {} data cache.", cache_name), | ||
)?, | ||
registry, | ||
)?, | ||
}) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn test_failure_insert_over_allocated_limit() { | ||
let mut cache = LRUCache::new("name", 10, None); | ||
// Should fail as base memory allocation for a string is 148 bytes. | ||
assert!(!cache.insert(0, "abcdefghijkl")); | ||
} | ||
|
||
#[test] | ||
fn test_work_allocated_size_limit() { | ||
let mut cache = LRUCache::new("name", 150, None); | ||
// Allocated memory will keep being 148 bytes for the three first insert. | ||
assert!(cache.insert(1, "a")); | ||
let memory_usage_step_1 = cache.inner.memory_usage(); | ||
assert!(cache.insert(2, "b")); | ||
assert!(cache.insert(3, "c")); | ||
// Memory allocation will try to reach 280 bytes, blocked by our limiter. Insertion should be | ||
// successful but first item should be removed from cache. | ||
assert!(cache.insert(4, "d")); | ||
let memory_usage_step_2 = cache.inner.memory_usage(); | ||
assert!(cache.get(&4).is_some()); | ||
assert!(cache.get(&1).is_none()); | ||
assert_eq!(memory_usage_step_1, memory_usage_step_2); | ||
} | ||
} |
Oops, something went wrong.
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.
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.
is there a way we can add tests for the metrics too?