From 142dbc0b2f724507235a78d28d93cb88e9fa62a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Ber=C4=8Di=C4=8D?= Date: Thu, 14 May 2020 18:19:50 +0200 Subject: [PATCH] runtime/storage/mkvs: Add method for checking local key existence --- .changelog/2938.feature.md | 4 + client/src/transaction/snapshot.rs | 4 + runtime/src/storage/mkvs/cache/cache.rs | 4 +- runtime/src/storage/mkvs/cache/lru_cache.rs | 10 +- runtime/src/storage/mkvs/mod.rs | 7 ++ runtime/src/storage/mkvs/tree/insert.rs | 2 +- runtime/src/storage/mkvs/tree/iterator.rs | 2 +- runtime/src/storage/mkvs/tree/lookup.rs | 32 +++++- runtime/src/storage/mkvs/tree/mkvs.rs | 5 + runtime/src/storage/mkvs/tree/remove.rs | 6 +- runtime/src/storage/mkvs/tree/tree_test.rs | 108 ++++++++++++++++++++ 11 files changed, 171 insertions(+), 13 deletions(-) create mode 100644 .changelog/2938.feature.md diff --git a/.changelog/2938.feature.md b/.changelog/2938.feature.md new file mode 100644 index 00000000000..d83714bc386 --- /dev/null +++ b/.changelog/2938.feature.md @@ -0,0 +1,4 @@ +runtime/storage/mkvs: Add method for checking local key existence + +Adds a method to probe the local cache for key existence, guaranteeing +that no remote syncing will be done. diff --git a/client/src/transaction/snapshot.rs b/client/src/transaction/snapshot.rs index a89fc9a9930..66f2f720142 100644 --- a/client/src/transaction/snapshot.rs +++ b/client/src/transaction/snapshot.rs @@ -107,6 +107,10 @@ impl MKVS for BlockSnapshot { MKVS::get(&self.mkvs, ctx, key) } + fn cache_contains_key(&self, ctx: Context, key: &[u8]) -> bool { + MKVS::cache_contains_key(&self.mkvs, ctx, key) + } + fn insert(&mut self, _ctx: Context, _key: &[u8], _value: &[u8]) -> Option> { unimplemented!("block snapshot is read-only"); } diff --git a/runtime/src/storage/mkvs/cache/cache.rs b/runtime/src/storage/mkvs/cache/cache.rs index 670afe8f998..daa6b7673f8 100644 --- a/runtime/src/storage/mkvs/cache/cache.rs +++ b/runtime/src/storage/mkvs/cache/cache.rs @@ -79,11 +79,13 @@ pub trait Cache { /// Dereference a node pointer into a concrete node object. /// /// Calling this method may invoke the underlying read syncer. + /// Giving a None fetcher forces the dereference to be local-only, + /// without invoking the read syncer. fn deref_node_ptr( &mut self, ctx: &Arc, ptr: NodePtrRef, - fetcher: F, + fetcher: Option, ) -> Fallible>; /// Perform a remote sync with the configured remote syncer. fn remote_sync( diff --git a/runtime/src/storage/mkvs/cache/lru_cache.rs b/runtime/src/storage/mkvs/cache/lru_cache.rs index 0065be67c87..10e75e38688 100644 --- a/runtime/src/storage/mkvs/cache/lru_cache.rs +++ b/runtime/src/storage/mkvs/cache/lru_cache.rs @@ -415,7 +415,7 @@ impl Cache for LRUCache { &mut self, ctx: &Arc, ptr: NodePtrRef, - fetcher: F, + fetcher: Option, ) -> Fallible> { let ptr_ref = ptr; let ptr = ptr_ref.borrow(); @@ -447,7 +447,13 @@ impl Cache for LRUCache { } // Node not available locally, fetch from read syncer. - self.remote_sync(ctx, ptr_ref.clone(), fetcher)?; + if let Some(fetcher) = fetcher { + self.remote_sync(ctx, ptr_ref.clone(), fetcher)?; + } else { + return Err(format_err!( + "mkvs: node to dereference not available locally and no fetcher provided" + )); + } let ptr = ptr_ref.borrow(); if ptr.node.is_none() { diff --git a/runtime/src/storage/mkvs/mod.rs b/runtime/src/storage/mkvs/mod.rs index af5a2094e01..b6a7fe0c19c 100644 --- a/runtime/src/storage/mkvs/mod.rs +++ b/runtime/src/storage/mkvs/mod.rs @@ -120,6 +120,13 @@ pub trait MKVS: Send + Sync { /// Fetch entry with given key. fn get(&self, ctx: Context, key: &[u8]) -> Option>; + /// Check if the local MKVS cache contains the given key. + /// + /// While get can be used to check if the MKVS as a whole contains + /// a given key, this function specifically guarantees that no remote + /// syncing will be invoked, only checking the local cache. + fn cache_contains_key(&self, ctx: Context, key: &[u8]) -> bool; + /// Update entry with given key. /// /// If the database did not have this key present, [`None`] is returned. diff --git a/runtime/src/storage/mkvs/tree/insert.rs b/runtime/src/storage/mkvs/tree/insert.rs index 3e78a68e7df..a0e9d59d29d 100644 --- a/runtime/src/storage/mkvs/tree/insert.rs +++ b/runtime/src/storage/mkvs/tree/insert.rs @@ -53,7 +53,7 @@ impl Tree { let node_ref = self.cache.borrow_mut().deref_node_ptr( ctx, ptr.clone(), - FetcherSyncGet::new(key, false), + Some(FetcherSyncGet::new(key, false)), )?; let (_, key_remainder) = key.split(bit_depth, key.bit_length()); diff --git a/runtime/src/storage/mkvs/tree/iterator.rs b/runtime/src/storage/mkvs/tree/iterator.rs index 133ba9f0c9b..99a67ff7ce1 100644 --- a/runtime/src/storage/mkvs/tree/iterator.rs +++ b/runtime/src/storage/mkvs/tree/iterator.rs @@ -195,7 +195,7 @@ impl<'tree> TreeIterator<'tree> { let node_ref = self.tree.cache.borrow_mut().deref_node_ptr( &self.ctx, ptr.clone(), - FetcherSyncIterate::new(&key, self.prefetch), + Some(FetcherSyncIterate::new(&key, self.prefetch)), )?; match classify_noderef!(?node_ref) { diff --git a/runtime/src/storage/mkvs/tree/lookup.rs b/runtime/src/storage/mkvs/tree/lookup.rs index bdf51d29de0..da0497c3e88 100644 --- a/runtime/src/storage/mkvs/tree/lookup.rs +++ b/runtime/src/storage/mkvs/tree/lookup.rs @@ -45,6 +45,19 @@ impl<'a> ReadSyncFetcher for FetcherSyncGet<'a> { impl Tree { /// Get an existing key. pub fn get(&self, ctx: Context, key: &[u8]) -> Fallible>> { + self._get_top(ctx, key, false) + } + + /// Check if the key exists in the local cache. + pub fn cache_contains_key(&self, ctx: Context, key: &[u8]) -> bool { + match self._get_top(ctx, key, true) { + Ok(Some(_)) => true, + Ok(None) => false, + Err(_) => false, + } + } + + fn _get_top(&self, ctx: Context, key: &[u8], check_only: bool) -> Fallible>> { let ctx = ctx.freeze(); let boxed_key = key.to_vec(); let pending_root = self.cache.borrow().get_pending_root(); @@ -57,7 +70,7 @@ impl Tree { // Remember where the path from root to target node ends (will end). self.cache.borrow_mut().mark_position(); - Ok(self._get(&ctx, pending_root, 0, &boxed_key, 0)?) + Ok(self._get(&ctx, pending_root, 0, &boxed_key, 0, check_only)?) } fn _get( @@ -67,11 +80,17 @@ impl Tree { bit_depth: Depth, key: &Key, depth: Depth, + check_only: bool, ) -> Fallible> { - let node_ref = - self.cache - .borrow_mut() - .deref_node_ptr(ctx, ptr, FetcherSyncGet::new(key, false))?; + let node_ref = self.cache.borrow_mut().deref_node_ptr( + ctx, + ptr, + if check_only { + None + } else { + Some(FetcherSyncGet::new(key, false)) + }, + )?; match classify_noderef!(?node_ref) { NodeKind::None => { @@ -90,6 +109,7 @@ impl Tree { bit_depth + n.label_bit_length, key, depth, + check_only, ); } @@ -106,6 +126,7 @@ impl Tree { bit_depth + n.label_bit_length, key, depth + 1, + check_only, ); } else { return self._get( @@ -114,6 +135,7 @@ impl Tree { bit_depth + n.label_bit_length, key, depth + 1, + check_only, ); } } diff --git a/runtime/src/storage/mkvs/tree/mkvs.rs b/runtime/src/storage/mkvs/tree/mkvs.rs index 1cc4e776339..160e80d702b 100644 --- a/runtime/src/storage/mkvs/tree/mkvs.rs +++ b/runtime/src/storage/mkvs/tree/mkvs.rs @@ -17,6 +17,11 @@ impl MKVS for Tree { self.get(ctx, key).unwrap() } + fn cache_contains_key(&self, ctx: Context, key: &[u8]) -> bool { + let _lock = self.lock.lock().unwrap(); + self.cache_contains_key(ctx, key) + } + fn insert(&mut self, ctx: Context, key: &[u8], value: &[u8]) -> Option> { let lock = self.lock.clone(); let _guard = lock.lock().unwrap(); diff --git a/runtime/src/storage/mkvs/tree/remove.rs b/runtime/src/storage/mkvs/tree/remove.rs index 2c9e854a4b9..278a83f62c7 100644 --- a/runtime/src/storage/mkvs/tree/remove.rs +++ b/runtime/src/storage/mkvs/tree/remove.rs @@ -54,7 +54,7 @@ impl Tree { let node_ref = self.cache.borrow_mut().deref_node_ptr( ctx, ptr.clone(), - FetcherSyncGet::new(key, true), + Some(FetcherSyncGet::new(key, true)), )?; match classify_noderef!(?node_ref) { @@ -106,12 +106,12 @@ impl Tree { remaining_left = self.cache.borrow_mut().deref_node_ptr( ctx, n.left.clone(), - FetcherSyncGet::new(key, true), + Some(FetcherSyncGet::new(key, true)), )?; remaining_right = self.cache.borrow_mut().deref_node_ptr( ctx, n.right.clone(), - FetcherSyncGet::new(key, true), + Some(FetcherSyncGet::new(key, true)), )?; } else { unreachable!("node kind is Internal"); diff --git a/runtime/src/storage/mkvs/tree/tree_test.rs b/runtime/src/storage/mkvs/tree/tree_test.rs index b08e6e1bfcc..d9e4e442505 100644 --- a/runtime/src/storage/mkvs/tree/tree_test.rs +++ b/runtime/src/storage/mkvs/tree/tree_test.rs @@ -66,6 +66,14 @@ fn test_basic() { .expect("insert"), None ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + false + ); let value = tree .get(Context::background(), key_zero) .expect("get") @@ -79,6 +87,14 @@ fn test_basic() { .as_slice(), value_zero ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + false + ); let value = tree .get(Context::background(), key_zero) .expect("get") @@ -107,6 +123,14 @@ fn test_basic() { .expect("insert"), None ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + true + ); let value = tree .get(Context::background(), key_one) .expect("get") @@ -120,6 +144,14 @@ fn test_basic() { .as_slice(), value_zero ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + true + ); let value = tree .get(Context::background(), key_zero) .expect("get") @@ -142,6 +174,14 @@ fn test_basic() { tree.remove(Context::background(), key_one).expect("remove"), None ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + false + ); assert_eq!(None, tree.get(Context::background(), key_one).expect("get")); let value = tree .get(Context::background(), key_zero) @@ -154,6 +194,14 @@ fn test_basic() { .expect("insert"), None ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + true + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + true + ); let value = tree .get(Context::background(), key_zero) .expect("get") @@ -233,6 +281,14 @@ fn test_basic() { assert_eq!(log[0].kind(), LogEntryKind::Delete); tree.remove(Context::background(), key_zero) .expect("remove"); + assert_eq!( + tree.cache_contains_key(Context::background(), key_zero), + false + ); + assert_eq!( + tree.cache_contains_key(Context::background(), key_one), + false + ); } #[test] @@ -256,6 +312,10 @@ fn test_long_keys() { } for i in 0..keys.len() { + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + true + ); let value = tree .get(Context::background(), keys[i].as_slice()) .expect("get") @@ -269,6 +329,10 @@ fn test_long_keys() { tree.remove(Context::background(), keys[i].as_slice()) .expect("remove"); + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + false + ); assert_eq!( None, tree.get(Context::background(), keys[i].as_slice()) @@ -296,9 +360,17 @@ fn test_empty_keys() { let empty_key = b""; let empty_value = b"empty value"; + assert_eq!( + tree.cache_contains_key(Context::background(), empty_key), + false + ); tree.insert(Context::background(), empty_key, empty_value) .expect("insert"); + assert_eq!( + tree.cache_contains_key(Context::background(), empty_key), + true + ); let value = tree .get(Context::background(), empty_key) .expect("get") @@ -308,6 +380,10 @@ fn test_empty_keys() { tree.remove(Context::background(), empty_key) .expect("remove"); + assert_eq!( + tree.cache_contains_key(Context::background(), empty_key), + false + ); assert_eq!( None, tree.get(Context::background(), empty_key).expect("get") @@ -466,6 +542,10 @@ fn test_remove() { let mut roots: Vec = Vec::new(); let (keys, values) = generate_key_value_pairs(); for i in 0..keys.len() { + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + false + ); tree.insert( Context::background(), keys[i].as_slice(), @@ -473,6 +553,10 @@ fn test_remove() { ) .expect("insert"); + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + true + ); let value = tree .get(Context::background(), keys[i].as_slice()) .expect("get") @@ -487,9 +571,17 @@ fn test_remove() { assert_eq!(format!("{:?}", roots[roots.len() - 1]), ALL_ITEMS_ROOT); for i in (1..keys.len()).rev() { + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + true + ); tree.remove(Context::background(), keys[i].as_slice()) .expect("remove"); + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + false + ); assert_eq!( None, tree.get(Context::background(), keys[i].as_slice()) @@ -509,6 +601,10 @@ fn test_remove() { // Now re-insert keys n..0, remove them in order 0..n. for i in (0..keys.len()).rev() { + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + false + ); tree.insert( Context::background(), keys[i].as_slice(), @@ -516,6 +612,10 @@ fn test_remove() { ) .expect("insert"); + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + true + ); let value = tree .get(Context::background(), keys[i].as_slice()) .expect("get") @@ -527,9 +627,17 @@ fn test_remove() { } for i in 0..keys.len() { + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + true + ); tree.remove(Context::background(), keys[i].as_slice()) .expect("remove"); + assert_eq!( + tree.cache_contains_key(Context::background(), keys[i].as_slice()), + false + ); assert_eq!( None, tree.get(Context::background(), keys[i].as_slice())