From 51d900d187ddc15f40368746b59e332858614aa9 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 3 Jun 2022 17:18:13 +0200 Subject: [PATCH] datastore: swap ConfigVersionCache with digest for change detection We got the digest available anyway, and it's only 16 bytes more to save (compared to last_generation and the recently removed last_time, both being 64 bit = 8 bytes each) Side benefit, we detect config changes made manually (e.g., `vim datacenter.cfg`) immediately. Note that we could restructure the maintenance mode checking to only be done after checking if there's a cached datastore, in which case using the generation could make sense to decide if we need to re-load it again before blindly loading the config anyway. As that's not only some (not exactly hard but not really trivial like a typo fix either) restructuring work but also means we'd lose the "detect manual changes" again I'd rather keep using the digest. Signed-off-by: Thomas Lamprecht --- pbs-config/src/config_version_cache.rs | 10 ++-------- pbs-datastore/src/datastore.rs | 23 +++++++++++------------ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/pbs-config/src/config_version_cache.rs b/pbs-config/src/config_version_cache.rs index b5e52486..3d570e93 100644 --- a/pbs-config/src/config_version_cache.rs +++ b/pbs-config/src/config_version_cache.rs @@ -26,6 +26,7 @@ struct ConfigVersionCacheDataInner { // Traffic control (traffic-control.cfg) generation/version. traffic_control_generation: AtomicUsize, // datastore (datastore.cfg) generation/version + // FIXME: remove with PBS 3.0 datastore_generation: AtomicUsize, // Add further atomics here } @@ -144,15 +145,8 @@ impl ConfigVersionCache { .fetch_add(1, Ordering::AcqRel); } - /// Returns the datastore generation number. - pub fn datastore_generation(&self) -> usize { - self.shmem - .data() - .datastore_generation - .load(Ordering::Acquire) - } - /// Increase the datastore generation number. + // FIXME: remove with PBS 3.0 or make actually useful again in datastore lookup pub fn increase_datastore_generation(&self) -> usize { self.shmem .data() diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 9f16beba..41dc41ff 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -21,7 +21,6 @@ use pbs_api_types::{ Authid, BackupNamespace, BackupType, ChunkOrder, DataStoreConfig, DatastoreTuning, GarbageCollectionStatus, HumanByte, Operation, UPID, }; -use pbs_config::ConfigVersionCache; use crate::backup_info::{BackupDir, BackupGroup}; use crate::chunk_store::ChunkStore; @@ -59,7 +58,7 @@ pub struct DataStoreImpl { last_gc_status: Mutex, verify_new: bool, chunk_order: ChunkOrder, - last_generation: usize, + last_digest: Option<[u8; 32]>, } impl DataStoreImpl { @@ -72,7 +71,7 @@ impl DataStoreImpl { last_gc_status: Mutex::new(GarbageCollectionStatus::default()), verify_new: false, chunk_order: ChunkOrder::None, - last_generation: 0, + last_digest: None, }) } } @@ -123,10 +122,9 @@ impl DataStore { name: &str, operation: Option, ) -> Result, Error> { - let version_cache = ConfigVersionCache::new()?; - let generation = version_cache.datastore_generation(); - - let (config, _digest) = pbs_config::datastore::config()?; + // we could use the ConfigVersionCache's generation for staleness detection, but we load + // the config anyway -> just use digest, additional benefit: manual changes get detected + let (config, digest) = pbs_config::datastore::config()?; let config: DataStoreConfig = config.lookup("datastore", name)?; if let Some(maintenance_mode) = config.get_maintenance_mode() { @@ -144,7 +142,8 @@ impl DataStore { // reuse chunk store so that we keep using the same process locker instance! let chunk_store = if let Some(datastore) = &entry { - if datastore.last_generation == generation { + 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 { inner: Arc::clone(datastore), operation, @@ -155,7 +154,7 @@ impl DataStore { Arc::new(ChunkStore::open(name, &config.path)?) }; - let datastore = DataStore::with_store_and_config(chunk_store, config, generation)?; + 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()); @@ -212,7 +211,7 @@ impl DataStore { let inner = Arc::new(Self::with_store_and_config( Arc::new(chunk_store), config, - 0, + None, )?); if let Some(operation) = operation { @@ -225,7 +224,7 @@ impl DataStore { fn with_store_and_config( chunk_store: Arc, config: DataStoreConfig, - last_generation: usize, + last_digest: Option<[u8; 32]>, ) -> Result { let mut gc_status_path = chunk_store.base_path(); gc_status_path.push(".gc-status"); @@ -254,7 +253,7 @@ impl DataStore { last_gc_status: Mutex::new(gc_status), verify_new: config.verify_new.unwrap_or(false), chunk_order, - last_generation, + last_digest, }) }