refactor(query): remove named cache (#16245)

* refactor(query): remove named cache

* refactor(query): remove named cache

* refactor(query): remove named cache

* refactor(query): remove named cache

Author: zhang2014

src/common/cache/src/cache.rs

@@ -23,15 +23,15 @@ use crate::Meter;
 pub trait Cache<K: Eq + Hash, V, M: Meter<K, V>> {
     /// Returns a reference to the value corresponding to the given key in the cache, if
     /// any.
-    fn get<'a, Q>(&'a mut self, k: &Q) -> Option<&'a V>
+    fn get<Q>(&mut self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
         Q: Hash + Eq + ?Sized;
 
     /// Returns a reference to the value corresponding to the key in the cache or `None` if it is
     /// not present in the cache. Unlike `get`, `peek` does not update the Cache state so the key's
     /// position will be unchanged.
-    fn peek<'a, Q>(&'a self, k: &Q) -> Option<&'a V>
+    fn peek<Q>(&self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
         Q: Hash + Eq + ?Sized;

src/common/cache/src/cache/lru.rs

@@ -211,26 +211,6 @@ impl<K: Eq + Hash, V, M: CountableMeter<K, V>> Cache<K, V, M> for LruCache<K, V,
         self.map.front()
     }
 
-    /// Checks if the map contains the given key.
-    ///
-    /// # Examples
-    ///
-    /// ```rust,ignore
-    /// use databend_common_cache::{Cache, LruCache};
-    ///
-    /// let mut cache = LruCache::new(1);
-    ///
-    /// cache.put(1, "a");
-    /// assert!(cache.contains(&1));
-    /// ```
-    fn contains<Q>(&self, key: &Q) -> bool
-    where
-        K: Borrow<Q>,
-        Q: Hash + Eq + ?Sized,
-    {
-        self.map.contains_key(key)
-    }
-
     /// Inserts a key-value pair into the cache. If the key already existed, the old value is
     /// returned.
     ///
@@ -314,6 +294,50 @@ impl<K: Eq + Hash, V, M: CountableMeter<K, V>> Cache<K, V, M> for LruCache<K, V,
         })
     }
 
+    /// Checks if the map contains the given key.
+    ///
+    /// # Examples
+    ///
+    /// ```rust,ignore
+    /// use databend_common_cache::{Cache, LruCache};
+    ///
+    /// let mut cache = LruCache::new(1);
+    ///
+    /// cache.put(1, "a");
+    /// assert!(cache.contains(&1));
+    /// ```
+    fn contains<Q>(&self, key: &Q) -> bool
+    where
+        K: Borrow<Q>,
+        Q: Hash + Eq + ?Sized,
+    {
+        self.map.contains_key(key)
+    }
+
+    /// Returns the number of key-value pairs in the cache.
+    fn len(&self) -> usize {
+        self.map.len()
+    }
+
+    /// Returns `true` if the cache contains no key-value pairs.
+    fn is_empty(&self) -> bool {
+        self.map.is_empty()
+    }
+
+    /// Returns the maximum size of the key-value pairs the cache can hold, as measured by the
+    /// `Meter` used by the cache.
+    ///
+    /// # Examples
+    ///
+    /// ```rust,ignore
+    /// use databend_common_cache::{Cache, LruCache};
+    /// let mut cache: LruCache<i32, &str> = LruCache::new(2);
+    /// assert_eq!(cache.capacity(), 2);
+    /// ```
+    fn capacity(&self) -> u64 {
+        self.max_capacity
+    }
+
     /// Sets the size of the key-value pairs the cache can hold, as measured by the `Meter` used by
     /// the cache.
     ///
@@ -355,11 +379,6 @@ impl<K: Eq + Hash, V, M: CountableMeter<K, V>> Cache<K, V, M> for LruCache<K, V,
         self.max_capacity = capacity;
     }
 
-    /// Returns the number of key-value pairs in the cache.
-    fn len(&self) -> usize {
-        self.map.len()
-    }
-
     /// Returns the size of all the key-value pairs in the cache, as measured by the `Meter` used
     /// by the cache.
     fn size(&self) -> u64 {
@@ -368,25 +387,6 @@ impl<K: Eq + Hash, V, M: CountableMeter<K, V>> Cache<K, V, M> for LruCache<K, V,
             .unwrap_or_else(|| self.map.len() as u64)
     }
 
-    /// Returns the maximum size of the key-value pairs the cache can hold, as measured by the
-    /// `Meter` used by the cache.
-    ///
-    /// # Examples
-    ///
-    /// ```rust,ignore
-    /// use databend_common_cache::{Cache, LruCache};
-    /// let mut cache: LruCache<i32, &str> = LruCache::new(2);
-    /// assert_eq!(cache.capacity(), 2);
-    /// ```
-    fn capacity(&self) -> u64 {
-        self.max_capacity
-    }
-
-    /// Returns `true` if the cache contains no key-value pairs.
-    fn is_empty(&self) -> bool {
-        self.map.is_empty()
-    }
-
     /// Removes all key-value pairs from the cache.
     fn clear(&mut self) {
         self.map.clear();

src/query/service/tests/it/storages/fuse/bloom_index_meta_size.rs

@@ -18,7 +18,7 @@ use std::sync::Arc;
 
 use chrono::Utc;
 use databend_common_base::base::tokio;
-use databend_common_cache::Cache;
+use databend_common_cache::LruCache;
 use databend_common_expression::types::Int32Type;
 use databend_common_expression::types::NumberDataType;
 use databend_common_expression::types::NumberScalar;
@@ -34,8 +34,9 @@ use databend_common_storages_fuse::statistics::gen_columns_statistics;
 use databend_common_storages_fuse::statistics::STATS_STRING_PREFIX_LEN;
 use databend_common_storages_fuse::FuseStorageFormat;
 use databend_query::test_kits::*;
-use databend_storages_common_cache::InMemoryCacheBuilder;
+use databend_storages_common_cache::CacheAccessor;
 use databend_storages_common_cache::InMemoryItemCacheHolder;
+use databend_storages_common_cache::Unit;
 use databend_storages_common_table_meta::meta::BlockMeta;
 use databend_storages_common_table_meta::meta::ColumnMeta;
 use databend_storages_common_table_meta::meta::ColumnStatistics;
@@ -176,9 +177,12 @@ async fn test_segment_info_size() -> databend_common_exception::Result<()> {
         scenario, pid, base_memory_usage
     );
 
-    let cache = InMemoryCacheBuilder::new_item_cache::<SegmentInfo>(cache_number as u64);
+    let cache = InMemoryItemCacheHolder::create(
+        String::from(""),
+        Unit::Count,
+        LruCache::new(cache_number as u64),
+    );
     {
-        let mut c = cache.write();
         for _ in 0..cache_number {
             let uuid = Uuid::new_v4();
             let block_metas = segment_info
@@ -188,7 +192,7 @@ async fn test_segment_info_size() -> databend_common_exception::Result<()> {
                 .collect::<Vec<_>>();
             let statistics = segment_info.summary.clone();
             let segment_info = SegmentInfo::new(block_metas, statistics);
-            (*c).put(format!("{}", uuid.simple()), Arc::new(segment_info));
+            cache.put(format!("{}", uuid.simple()), Arc::new(segment_info));
         }
     }
     show_memory_usage("SegmentInfoCache", base_memory_usage, cache_number);
@@ -220,17 +224,20 @@ async fn test_segment_raw_bytes_size() -> databend_common_exception::Result<()>
         scenario, pid, base_memory_usage
     );
 
-    let cache = InMemoryCacheBuilder::new_item_cache::<Vec<u8>>(cache_number as u64);
-    {
-        let mut c = cache.write();
-        for _ in 0..cache_number {
-            let uuid = Uuid::new_v4();
-            (*c).put(
-                format!("{}", uuid.simple()),
-                Arc::new(segment_info_bytes.clone()),
-            );
-        }
+    let cache = InMemoryItemCacheHolder::create(
+        String::from(""),
+        Unit::Count,
+        LruCache::new(cache_number as u64),
+    );
+
+    for _ in 0..cache_number {
+        let uuid = Uuid::new_v4();
+        cache.put(
+            format!("{}", uuid.simple()),
+            Arc::new(segment_info_bytes.clone()),
+        );
     }
+
     show_memory_usage(
         "SegmentInfoCache(raw bytes Vec<u8>)",
         base_memory_usage,
@@ -265,13 +272,15 @@ async fn test_segment_raw_repr_bytes_size() -> databend_common_exception::Result
         scenario, pid, base_memory_usage
     );
 
-    let cache = InMemoryCacheBuilder::new_item_cache::<CompactSegmentInfo>(cache_number as u64);
-    {
-        let mut c = cache.write();
-        for _ in 0..cache_number {
-            let uuid = Uuid::new_v4();
-            (*c).put(format!("{}", uuid.simple()), Arc::new(segment_raw.clone()));
-        }
+    let cache = InMemoryItemCacheHolder::create(
+        String::from(""),
+        Unit::Count,
+        LruCache::new(cache_number as u64),
+    );
+
+    for _ in 0..cache_number {
+        let uuid = Uuid::new_v4();
+        cache.put(format!("{}", uuid.simple()), Arc::new(segment_raw.clone()));
     }
     show_memory_usage(
         "SegmentInfoCache (compact repr)",
@@ -363,10 +372,9 @@ fn build_test_segment_info(
 #[allow(dead_code)]
 fn populate_cache<T>(cache: &InMemoryItemCacheHolder<T>, item: T, num_cache: usize)
 where T: Clone {
-    let mut c = cache.write();
     for _ in 0..num_cache {
         let uuid = Uuid::new_v4();
-        (*c).put(
+        cache.put(
             format!("{}", uuid.simple()),
             std::sync::Arc::new(item.clone()),
         );

src/query/storages/common/cache/src/cache.rs

@@ -14,12 +14,10 @@
 
 use std::fmt::Display;
 use std::fmt::Formatter;
-use std::hash::Hash;
 use std::sync::Arc;
 
 use databend_common_cache::Count;
 use databend_common_cache::CountableMeter;
-use databend_common_metrics::cache::*;
 
 #[derive(Copy, Clone, Debug)]
 pub enum Unit {
@@ -37,133 +35,22 @@ impl Display for Unit {
 }
 
 // The cache accessor, crate users usually working on this interface while manipulating caches
-pub trait CacheAccessor<K, V, M = Count>
-where
-    K: Eq + Hash,
-    M: CountableMeter<K, Arc<V>>,
-{
-    fn get<Q: AsRef<str>>(&self, k: Q) -> Option<Arc<V>>;
-    fn put(&self, key: K, value: Arc<V>);
+pub trait CacheAccessor {
+    type V;
+    type M: CountableMeter<String, Arc<Self::V>> = Count;
+
+    fn get<Q: AsRef<str>>(&self, k: Q) -> Option<Arc<Self::V>>;
+    fn get_sized<Q: AsRef<str>>(&self, k: Q, len: u64) -> Option<Arc<Self::V>>;
+
+    fn put(&self, key: String, value: Arc<Self::V>);
     fn evict(&self, k: &str) -> bool;
     fn contains_key(&self, k: &str) -> bool;
     fn size(&self) -> u64;
     fn capacity(&self) -> u64;
-    fn set_capacity(&self, capacity: u64);
     fn len(&self) -> usize;
     fn is_empty(&self) -> bool {
         self.len() == 0
     }
-}
-
-/// Helper trait to convert a Cache into NamedCache
-pub trait Named
-where Self: Sized
-{
-    fn name_with(self, name: impl Into<String>, unit: Unit) -> NamedCache<Self> {
-        NamedCache {
-            name: name.into(),
-            cache: self,
-            unit,
-        }
-    }
-}
-
-impl<T> Named for T where T: Sized + Clone {}
-
-/// A named cache that with embedded metrics logging
-#[derive(Clone)]
-pub struct NamedCache<C> {
-    name: String,
-    unit: Unit,
-    cache: C,
-}
-
-impl<C> NamedCache<C> {
-    #[inline]
-    pub fn name(&self) -> &str {
-        &self.name
-    }
-    #[inline]
-    pub fn unit(&self) -> Unit {
-        self.unit
-    }
-}
-
-pub trait CacheAccessorExt<K, V, M> {
-    fn get_with_len<Q: AsRef<str>>(&self, k: Q, len: u64) -> Option<Arc<V>>;
-}
-
-impl<K, V, M, C> CacheAccessorExt<K, V, M> for NamedCache<C>
-where
-    C: CacheAccessor<K, V, M>,
-    K: Eq + Hash,
-    M: CountableMeter<K, Arc<V>>,
-{
-    fn get_with_len<Q: AsRef<str>>(&self, k: Q, len: u64) -> Option<Arc<V>> {
-        let r = self.get(k);
-        if r.is_none() {
-            metrics_inc_cache_miss_count(len, &self.name);
-        }
-        r
-    }
-}
 
-impl<K, V, M, C> CacheAccessorExt<K, V, M> for Option<NamedCache<C>>
-where
-    C: CacheAccessor<K, V, M>,
-    K: Eq + Hash,
-    M: CountableMeter<K, Arc<V>>,
-{
-    fn get_with_len<Q: AsRef<str>>(&self, k: Q, len: u64) -> Option<Arc<V>> {
-        self.as_ref().and_then(|cache| cache.get_with_len(k, len))
-    }
-}
-
-impl<K, V, M, C> CacheAccessor<K, V, M> for NamedCache<C>
-where
-    C: CacheAccessor<K, V, M>,
-    K: Eq + Hash,
-    M: CountableMeter<K, Arc<V>>,
-{
-    fn get<Q: AsRef<str>>(&self, k: Q) -> Option<Arc<V>> {
-        metrics_inc_cache_access_count(1, &self.name);
-        match self.cache.get(k) {
-            None => {
-                metrics_inc_cache_miss_count(1, &self.name);
-                None
-            }
-            v @ Some(_) => {
-                metrics_inc_cache_hit_count(1, &self.name);
-                v
-            }
-        }
-    }
-
-    fn put(&self, key: K, value: Arc<V>) {
-        self.cache.put(key, value)
-    }
-
-    fn evict(&self, k: &str) -> bool {
-        self.cache.evict(k)
-    }
-
-    fn size(&self) -> u64 {
-        self.cache.size()
-    }
-
-    fn capacity(&self) -> u64 {
-        self.cache.capacity()
-    }
-
-    fn set_capacity(&self, capacity: u64) {
-        self.cache.set_capacity(capacity)
-    }
-
-    fn len(&self) -> usize {
-        self.cache.len()
-    }
-
-    fn contains_key(&self, k: &str) -> bool {
-        self.cache.contains_key(k)
-    }
+    fn name(&self) -> &str;
 }