diff --git a/firewood/src/manager.rs b/firewood/src/manager.rs index e85123af4..51684e709 100644 --- a/firewood/src/manager.rs +++ b/firewood/src/manager.rs @@ -14,7 +14,9 @@ use typed_builder::TypedBuilder; use crate::v2::api::HashKey; -use storage::{Committed, FileBacked, ImmutableProposal, NodeStore, Parentable, TrieHash}; +use storage::{ + CacheReadStrategy, Committed, FileBacked, ImmutableProposal, NodeStore, Parentable, TrieHash, +}; #[derive(Clone, Debug, TypedBuilder)] /// Revision manager configuratoin @@ -29,6 +31,9 @@ pub struct RevisionManagerConfig { #[builder(default_code = "NonZero::new(40000).expect(\"non-zero\")")] free_list_cache_size: NonZero, + + #[builder(default = CacheReadStrategy::WritesOnly)] + cache_read_strategy: CacheReadStrategy, } type CommittedRevision = Arc>; @@ -73,6 +78,7 @@ impl RevisionManager { config.node_cache_size, config.free_list_cache_size, truncate, + config.cache_read_strategy, )?); let nodestore = match truncate { true => Arc::new(NodeStore::new_empty_committed(storage.clone())?), diff --git a/storage/src/lib.rs b/storage/src/lib.rs index e8ed90873..8a413b858 100644 --- a/storage/src/lib.rs +++ b/storage/src/lib.rs @@ -34,3 +34,19 @@ pub use linear::filebacked::FileBacked; pub use linear::memory::MemStore; pub use trie_hash::TrieHash; + +/// The strategy for caching nodes that are read +/// from the storage layer. Generally, we only want to +/// cache write operations, but for some read-heavy workloads +/// you can enable caching of branch reads or all reads. +#[derive(Clone, Debug)] +pub enum CacheReadStrategy { + /// Only cache writes (no reads will be cached) + WritesOnly, + + /// Cache branch reads (reads that are not leaf nodes) + BranchReads, + + /// Cache all reads (leaves and branches) + All, +} diff --git a/storage/src/linear/filebacked.rs b/storage/src/linear/filebacked.rs index 9b93a907d..47eae344d 100644 --- a/storage/src/linear/filebacked.rs +++ b/storage/src/linear/filebacked.rs @@ -16,7 +16,7 @@ use std::sync::{Arc, Mutex}; use lru::LruCache; use metrics::counter; -use crate::{LinearAddress, Node}; +use crate::{CacheReadStrategy, LinearAddress, Node}; use super::{ReadableStorage, WritableStorage}; @@ -26,6 +26,7 @@ pub struct FileBacked { fd: File, cache: Mutex>>, free_list_cache: Mutex>>, + cache_read_strategy: CacheReadStrategy, } impl FileBacked { @@ -35,6 +36,7 @@ impl FileBacked { node_cache_size: NonZero, free_list_cache_size: NonZero, truncate: bool, + cache_read_strategy: CacheReadStrategy, ) -> Result { let fd = OpenOptions::new() .read(true) @@ -47,6 +49,7 @@ impl FileBacked { fd, cache: Mutex::new(LruCache::new(node_cache_size)), free_list_cache: Mutex::new(LruCache::new(free_list_cache_size)), + cache_read_strategy, }) } } @@ -74,6 +77,28 @@ impl ReadableStorage for FileBacked { counter!("firewood.cache.freelist", "type" => if cached.is_some() { "hit" } else { "miss" }).increment(1); cached } + + fn cache_read_strategy(&self) -> &CacheReadStrategy { + &self.cache_read_strategy + } + + fn cache_node(&self, addr: LinearAddress, node: Arc) { + match self.cache_read_strategy { + CacheReadStrategy::WritesOnly => { + // we don't cache reads + } + CacheReadStrategy::All => { + let mut guard = self.cache.lock().expect("poisoned lock"); + guard.put(addr, node); + } + CacheReadStrategy::BranchReads => { + if !node.is_leaf() { + let mut guard = self.cache.lock().expect("poisoned lock"); + guard.put(addr, node); + } + } + } + } } impl WritableStorage for FileBacked { @@ -169,6 +194,7 @@ mod test { NonZero::new(10).unwrap(), NonZero::new(10).unwrap(), false, + CacheReadStrategy::WritesOnly, ) .unwrap(); let mut reader = fb.stream_from(0).unwrap(); @@ -208,6 +234,7 @@ mod test { NonZero::new(10).unwrap(), NonZero::new(10).unwrap(), false, + CacheReadStrategy::WritesOnly, ) .unwrap(); let mut reader = fb.stream_from(0).unwrap(); diff --git a/storage/src/linear/mod.rs b/storage/src/linear/mod.rs index 824b0e0cb..87340f5e8 100644 --- a/storage/src/linear/mod.rs +++ b/storage/src/linear/mod.rs @@ -22,7 +22,7 @@ use std::io::{Error, Read}; use std::num::NonZero; use std::sync::Arc; -use crate::{LinearAddress, Node}; +use crate::{CacheReadStrategy, LinearAddress, Node}; pub(super) mod filebacked; pub mod memory; @@ -51,6 +51,14 @@ pub trait ReadableStorage: Debug + Sync + Send { fn free_list_cache(&self, _addr: LinearAddress) -> Option> { None } + + /// Return the cache read strategy for this readable storage + fn cache_read_strategy(&self) -> &CacheReadStrategy { + &CacheReadStrategy::WritesOnly + } + + /// Cache a node for future reads + fn cache_node(&self, _addr: LinearAddress, _node: Arc) {} } /// Trait for writable storage. diff --git a/storage/src/nodestore.rs b/storage/src/nodestore.rs index 8f067ea58..f63614fff 100644 --- a/storage/src/nodestore.rs +++ b/storage/src/nodestore.rs @@ -61,7 +61,7 @@ use std::sync::Arc; use crate::hashednode::hash_node; use crate::node::{ByteCounter, Node}; -use crate::{Child, FileBacked, Path, ReadableStorage, TrieHash}; +use crate::{CacheReadStrategy, Child, FileBacked, Path, ReadableStorage, TrieHash}; use super::linear::WritableStorage; @@ -217,13 +217,24 @@ impl NodeStore { debug_assert!(addr.get() % 8 == 0); - let addr = addr.get() + 1; // skip the length byte + let actual_addr = addr.get() + 1; // skip the length byte let _span = LocalSpan::enter_with_local_parent("read_and_deserialize"); - let area_stream = self.storage.stream_from(addr)?; - let node = Node::from_reader(area_stream)?; - Ok(node.into()) + let area_stream = self.storage.stream_from(actual_addr)?; + let node = Arc::new(Node::from_reader(area_stream)?); + match self.storage.cache_read_strategy() { + CacheReadStrategy::All => { + self.storage.cache_node(addr, node.clone()); + } + CacheReadStrategy::BranchReads => { + if !node.is_leaf() { + self.storage.cache_node(addr, node.clone()); + } + } + CacheReadStrategy::WritesOnly => {} + } + Ok(node) } }