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 <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2022-06-03 17:18:13 +02:00
parent 519ca9d010
commit 51d900d187
2 changed files with 13 additions and 20 deletions

View File

@ -26,6 +26,7 @@ struct ConfigVersionCacheDataInner {
// Traffic control (traffic-control.cfg) generation/version. // Traffic control (traffic-control.cfg) generation/version.
traffic_control_generation: AtomicUsize, traffic_control_generation: AtomicUsize,
// datastore (datastore.cfg) generation/version // datastore (datastore.cfg) generation/version
// FIXME: remove with PBS 3.0
datastore_generation: AtomicUsize, datastore_generation: AtomicUsize,
// Add further atomics here // Add further atomics here
} }
@ -144,15 +145,8 @@ impl ConfigVersionCache {
.fetch_add(1, Ordering::AcqRel); .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. /// 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 { pub fn increase_datastore_generation(&self) -> usize {
self.shmem self.shmem
.data() .data()

View File

@ -21,7 +21,6 @@ use pbs_api_types::{
Authid, BackupNamespace, BackupType, ChunkOrder, DataStoreConfig, DatastoreTuning, Authid, BackupNamespace, BackupType, ChunkOrder, DataStoreConfig, DatastoreTuning,
GarbageCollectionStatus, HumanByte, Operation, UPID, GarbageCollectionStatus, HumanByte, Operation, UPID,
}; };
use pbs_config::ConfigVersionCache;
use crate::backup_info::{BackupDir, BackupGroup}; use crate::backup_info::{BackupDir, BackupGroup};
use crate::chunk_store::ChunkStore; use crate::chunk_store::ChunkStore;
@ -59,7 +58,7 @@ pub struct DataStoreImpl {
last_gc_status: Mutex<GarbageCollectionStatus>, last_gc_status: Mutex<GarbageCollectionStatus>,
verify_new: bool, verify_new: bool,
chunk_order: ChunkOrder, chunk_order: ChunkOrder,
last_generation: usize, last_digest: Option<[u8; 32]>,
} }
impl DataStoreImpl { impl DataStoreImpl {
@ -72,7 +71,7 @@ impl DataStoreImpl {
last_gc_status: Mutex::new(GarbageCollectionStatus::default()), last_gc_status: Mutex::new(GarbageCollectionStatus::default()),
verify_new: false, verify_new: false,
chunk_order: ChunkOrder::None, chunk_order: ChunkOrder::None,
last_generation: 0, last_digest: None,
}) })
} }
} }
@ -123,10 +122,9 @@ impl DataStore {
name: &str, name: &str,
operation: Option<Operation>, operation: Option<Operation>,
) -> Result<Arc<DataStore>, Error> { ) -> Result<Arc<DataStore>, Error> {
let version_cache = ConfigVersionCache::new()?; // we could use the ConfigVersionCache's generation for staleness detection, but we load
let generation = version_cache.datastore_generation(); // the config anyway -> just use digest, additional benefit: manual changes get detected
let (config, digest) = pbs_config::datastore::config()?;
let (config, _digest) = pbs_config::datastore::config()?;
let config: DataStoreConfig = config.lookup("datastore", name)?; let config: DataStoreConfig = config.lookup("datastore", name)?;
if let Some(maintenance_mode) = config.get_maintenance_mode() { 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! // reuse chunk store so that we keep using the same process locker instance!
let chunk_store = if let Some(datastore) = &entry { 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 { return Ok(Arc::new(Self {
inner: Arc::clone(datastore), inner: Arc::clone(datastore),
operation, operation,
@ -155,7 +154,7 @@ impl DataStore {
Arc::new(ChunkStore::open(name, &config.path)?) 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); let datastore = Arc::new(datastore);
datastore_cache.insert(name.to_string(), datastore.clone()); datastore_cache.insert(name.to_string(), datastore.clone());
@ -212,7 +211,7 @@ impl DataStore {
let inner = Arc::new(Self::with_store_and_config( let inner = Arc::new(Self::with_store_and_config(
Arc::new(chunk_store), Arc::new(chunk_store),
config, config,
0, None,
)?); )?);
if let Some(operation) = operation { if let Some(operation) = operation {
@ -225,7 +224,7 @@ impl DataStore {
fn with_store_and_config( fn with_store_and_config(
chunk_store: Arc<ChunkStore>, chunk_store: Arc<ChunkStore>,
config: DataStoreConfig, config: DataStoreConfig,
last_generation: usize, last_digest: Option<[u8; 32]>,
) -> Result<DataStoreImpl, Error> { ) -> Result<DataStoreImpl, Error> {
let mut gc_status_path = chunk_store.base_path(); let mut gc_status_path = chunk_store.base_path();
gc_status_path.push(".gc-status"); gc_status_path.push(".gc-status");
@ -254,7 +253,7 @@ impl DataStore {
last_gc_status: Mutex::new(gc_status), last_gc_status: Mutex::new(gc_status),
verify_new: config.verify_new.unwrap_or(false), verify_new: config.verify_new.unwrap_or(false),
chunk_order, chunk_order,
last_generation, last_digest,
}) })
} }