starknet_patricia_storage: always cache writes in cached storage

Author: nimrod-starkware

crates/apollo_deployments/resources/app_configs/committer_config.json

@@ -2,7 +2,6 @@
     "committer_config.db_path": "/data/committer",
     "committer_config.reader_config.warn_on_trivial_modifications": false,
     "committer_config.reader_config.build_storage_tries_concurrently": false,
-    "committer_config.storage_config.cache_on_write": true,
     "committer_config.storage_config.cache_size": 1000000,
     "committer_config.storage_config.include_inner_stats": true,
     "committer_config.storage_config.inner_storage_config.bloom_filter_bits": 10,

crates/apollo_deployments/resources/app_configs/replacer_committer_config.json

@@ -2,7 +2,6 @@
   "committer_config.db_path": "/data/committer",
   "committer_config.reader_config.warn_on_trivial_modifications": false,
   "committer_config.reader_config.build_storage_tries_concurrently": false,
-  "committer_config.storage_config.cache_on_write": true,
   "committer_config.storage_config.cache_size": "$$$_COMMITTER_CONFIG-STORAGE_CONFIG-CACHE_SIZE_$$$",
   "committer_config.storage_config.include_inner_stats": true,
   "committer_config.storage_config.inner_storage_config.bloom_filter_bits": 10,

crates/apollo_node/resources/config_schema.json

@@ -514,11 +514,6 @@
     "privacy": "Public",
     "value": false
   },
-  "committer_config.storage_config.cache_on_write": {
-    "description": "If true, the cache is updated on write operations",
-    "privacy": "Public",
-    "value": true
-  },
   "committer_config.storage_config.cache_size": {
     "description": "Max number of entries in the cache",
     "privacy": "Public",

crates/starknet_committer_cli/src/args.rs

@@ -231,7 +231,6 @@ impl<InnerStorageArgs: StorageFromArgs + Sync> StorageFromArgs
     fn storage_config(&self) -> <Self::Storage as Storage>::Config {
         CachedStorageConfig {
             cache_size: NonZeroUsize::new(self.cache_size).unwrap(),
-            cache_on_write: true,
             include_inner_stats: self.include_inner_stats,
             inner_storage_config: self.storage_args.storage_config(),
         }

crates/starknet_patricia_storage/src/map_storage.rs

@@ -76,7 +76,6 @@ impl Storage for MapStorage {
 pub struct CachedStorage<S: Storage> {
     pub storage: S,
     pub cache: LruCache<DbKey, Option<DbValue>>,
-    pub cache_on_write: bool,
     reads: AtomicU64,
     cached_reads: AtomicU64,
     writes: u128,
@@ -88,9 +87,6 @@ pub struct CachedStorageConfig<InnerStorageConfig: StorageConfigTrait> {
     // Max number of entries in the cache.
     pub cache_size: NonZeroUsize,
 
-    // If true, the cache is updated on write operations even if the value is not in the cache.
-    pub cache_on_write: bool,
-
     // If true, the inner stats are included when collecting statistics.
     pub include_inner_stats: bool,
 
@@ -102,7 +98,6 @@ impl<InnerStorageConfig: StorageConfigTrait> Default for CachedStorageConfig<Inn
     fn default() -> Self {
         Self {
             cache_size: NonZeroUsize::new(DEFAULT_CACHE_SIZE).unwrap(),
-            cache_on_write: true,
             include_inner_stats: true,
             inner_storage_config: InnerStorageConfig::default(),
         }
@@ -126,12 +121,6 @@ impl<InnerStorageConfig: StorageConfigTrait> SerializeConfig
                 "Max number of entries in the cache",
                 ParamPrivacyInput::Public,
             ),
-            ser_param(
-                "cache_on_write",
-                &self.cache_on_write,
-                "If true, the cache is updated on write operations",
-                ParamPrivacyInput::Public,
-            ),
             ser_param(
                 "include_inner_stats",
                 &self.include_inner_stats,
@@ -204,20 +193,13 @@ impl<S: Storage> CachedStorage<S> {
         Self {
             storage,
             cache: LruCache::new(config.cache_size),
-            cache_on_write: config.cache_on_write,
             reads: AtomicU64::new(0),
             cached_reads: AtomicU64::new(0),
             writes: 0,
             include_inner_stats: config.include_inner_stats,
         }
     }
 
-    fn update_cached_value(&mut self, key: &DbKey, value: &DbValue) {
-        if self.cache_on_write || self.cache.contains(key) {
-            self.cache.put(key.clone(), Some(value.clone()));
-        }
-    }
-
     pub fn total_writes(&self) -> u128 {
         self.writes
     }
@@ -247,7 +229,7 @@ impl<S: Storage> Storage for CachedStorage<S> {
     async fn set(&mut self, key: DbKey, value: DbValue) -> PatriciaStorageResult<()> {
         self.writes += 1;
         self.storage.set(key.clone(), value.clone()).await?;
-        self.update_cached_value(&key, &value);
+        self.cache.put(key, Some(value));
         Ok(())
     }
 
@@ -282,9 +264,7 @@ impl<S: Storage> Storage for CachedStorage<S> {
     async fn mset(&mut self, key_to_value: DbHashMap) -> PatriciaStorageResult<()> {
         self.writes += u128::try_from(key_to_value.len()).expect("usize should fit in u128");
         self.storage.mset(key_to_value.clone()).await?;
-        key_to_value.iter().for_each(|(key, value)| {
-            self.update_cached_value(key, value);
-        });
+        self.flush_to_cache(key_to_value);
         Ok(())
     }
 

crates/starknet_patricia_storage/src/map_storage_test.rs

@@ -11,7 +11,6 @@ use crate::storage_trait::{DbKey, DbValue, EmptyStorageConfig, Storage};
 #[case::cached_storage(
     CachedStorage::new(MapStorage::default(), CachedStorageConfig {
         cache_size: NonZeroUsize::new(2).unwrap(),
-        cache_on_write: true,
         include_inner_stats: false,
         inner_storage_config: EmptyStorageConfig::default(),
     })