[pbs-devel] [PATCH proxmox-backup v4 02/22] datastore: make dropping from cache more efficient

[Hannes Laimer <h.laimer@proxmox.com>]

... by not having to read the whole config file on every drop,
but assume calling `update-datastore-cache` through the scoket implies
it should be dropped and doing the checking on the sending side.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 60 +++++++++++++++-------------------
 src/api2/config/datastore.rs   |  8 +++--
 2 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 0685cc84..a7fe3b8c 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -34,7 +34,7 @@ use crate::task_tracking::{self, update_active_operations};
 use crate::DataBlob;
 
 lazy_static! {
-    static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStoreImpl>>> =
+    static ref DATASTORE_MAP: Mutex<HashMap<String, (Arc<DataStoreImpl>, bool)>> =
         Mutex::new(HashMap::new());
 }
 
@@ -111,24 +111,15 @@ impl Drop for DataStore {
                     last_task = updated_operations.read + updated_operations.write == 0;
                 }
             }
-
-            // remove datastore from cache iff
-            //  - last task finished, and
-            //  - datastore is in a maintenance mode that mandates it
-            let remove_from_cache = last_task
-                && pbs_config::datastore::config()
-                    .and_then(|(s, _)| s.lookup::<DataStoreConfig>("datastore", self.name()))
-                    .map_or(false, |c| {
-                        c.get_maintenance_mode().map_or(false, |m| m.is_offline())
-                    });
-
-            if remove_from_cache {
-                DATASTORE_MAP.lock().unwrap().remove(self.name());
+            if last_task {
+                let mut cache = DATASTORE_MAP.lock().unwrap();
+                if let Some((_, true)) = cache.get(self.name()) {
+                    cache.remove(self.name());
+                }
             }
         }
     }
 }
-
 impl DataStore {
     // This one just panics on everything
     #[doc(hidden)]
@@ -169,7 +160,7 @@ impl DataStore {
         let entry = datastore_cache.get(name);
 
         // reuse chunk store so that we keep using the same process locker instance!
-        let chunk_store = if let Some(datastore) = &entry {
+        let (chunk_store, drop_from_cache) = if let Some((datastore, drop_from_cache)) = &entry {
             let last_digest = datastore.last_digest.as_ref();
             if let Some(true) = last_digest.map(|last_digest| last_digest == &digest) {
                 return Ok(Arc::new(Self {
@@ -177,23 +168,26 @@ impl DataStore {
                     operation,
                 }));
             }
-            Arc::clone(&datastore.chunk_store)
+            (Arc::clone(&datastore.chunk_store), *drop_from_cache)
         } else {
             let tuning: DatastoreTuning = serde_json::from_value(
                 DatastoreTuning::API_SCHEMA
                     .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
             )?;
-            Arc::new(ChunkStore::open(
-                name,
-                &config.path,
-                tuning.sync_level.unwrap_or_default(),
-            )?)
+            (
+                Arc::new(ChunkStore::open(
+                    name,
+                    &config.path,
+                    tuning.sync_level.unwrap_or_default(),
+                )?),
+                false,
+            )
         };
 
         let datastore = DataStore::with_store_and_config(chunk_store, config, Some(digest))?;
 
         let datastore = Arc::new(datastore);
-        datastore_cache.insert(name.to_string(), datastore.clone());
+        datastore_cache.insert(name.to_string(), (datastore.clone(), drop_from_cache));
 
         Ok(Arc::new(Self {
             inner: datastore,
@@ -211,20 +205,18 @@ impl DataStore {
         Ok(())
     }
 
-    /// trigger clearing cache entry based on maintenance mode. Entry will only
-    /// be cleared iff there is no other task running, if there is, the end of the
+    /// trigger clearing of cache entry. Entry will only be cleared iff
+    /// there is no other task running, if there is, the end of the
     /// last running task will trigger the clearing of the cache entry.
     pub fn update_datastore_cache(name: &str) -> Result<(), Error> {
-        let (config, _digest) = pbs_config::datastore::config()?;
-        let datastore: DataStoreConfig = config.lookup("datastore", name)?;
-        if datastore
-            .get_maintenance_mode()
-            .map_or(false, |m| m.is_offline())
-        {
-            // the datastore drop handler does the checking if tasks are running and clears the
-            // cache entry, so we just have to trigger it here
-            let _ = DataStore::lookup_datastore(name, Some(Operation::Lookup));
+        let _store = DataStore::lookup_datastore(name, Some(Operation::Lookup));
+
+        let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
+        if let Some((_, drop_from_cache)) = datastore_cache.get_mut(name) {
+            *drop_from_cache = true;
         }
+        drop(datastore_cache);
+        drop(_store);
 
         Ok(())
     }
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 3081e1f4..0b3d92f9 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -389,10 +389,12 @@ pub fn update_datastore(
         data.tuning = update.tuning;
     }
 
-    let mut maintenance_mode_changed = false;
+    let mut drop_store_from_cache = false;
     if update.maintenance_mode.is_some() {
-        maintenance_mode_changed = data.maintenance_mode != update.maintenance_mode;
         data.maintenance_mode = update.maintenance_mode;
+        drop_store_from_cache = data
+            .get_maintenance_mode()
+            .map_or(false, |m| m.is_offline());
     }
 
     config.set_data(&name, "datastore", &data)?;
@@ -406,7 +408,7 @@ pub fn update_datastore(
     }
 
     // tell the proxy it might have to clear a cache entry
-    if maintenance_mode_changed {
+    if drop_store_from_cache {
         tokio::spawn(async move {
             if let Ok(proxy_pid) =
                 proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
-- 
2.39.2