[pbs-devel] [PATCH proxmox-backup v4 02/22] datastore: make dropping from cache more efficient
Dietmar Maurer
dietmar at proxmox.com
Wed Apr 17 11:37:33 CEST 2024
First, I think the commit message it totally misleading. This is not for efficiency, instead, this replaces the former MaintenanceType::Unplugged functionality?
But we still have MaintenanceType::Delete, and I am quite unsure it we should handle those states differently.
> On 16.4.2024 17:23 CEST Hannes Laimer <h.laimer at proxmox.com> wrote:
>
>
> ... 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 at 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
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
More information about the pbs-devel
mailing list