From ec44c3113ba866cfe62aed37f35af98f0e38dd3d Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Thu, 2 Jun 2022 17:59:58 +0200 Subject: [PATCH] backport "datastore: lookup: reuse ChunkStore on stale datastore re-open" Backport of commit 0bd9c87010e25634ea6a91c65c2ff8088372340d When re-opening a datastore due to the cached entry being stale (only on verify-new config change). On datastore open the chunk store was also re-opened, which in turn creates a new ProcessLocker, loosing any existing shared lock which can cause conflicts between long running (24h+) backups and GC. To fix this, reuse the existing ChunkStore, and thus its ProcessLocker, when creating a up-to-date datastore instance on lookup, since only the datastore config should be reloaded. This is fine as the ChunkStore path is not updatable over our API. Note that this is a precaution backport, the underlying issue this fixes is relatively unlikely to cause any trouble in the 1.x branch due to not often re-opening the datastore. Originally-by: Dominik Csapak Signed-off-by: Thomas Lamprecht --- src/backup/datastore.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs index a0cf50b2..80d5c1d4 100644 --- a/src/backup/datastore.rs +++ b/src/backup/datastore.rs @@ -52,16 +52,20 @@ impl DataStore { let mut map = DATASTORE_MAP.lock().unwrap(); - if let Some(datastore) = map.get(name) { + // reuse chunk store so that we keep using the same process locker instance! + let chunk_store = if let Some(datastore) = map.get(name) { // Compare Config - if changed, create new Datastore object! if datastore.chunk_store.base == path && datastore.verify_new == config.verify_new.unwrap_or(false) { return Ok(datastore.clone()); } - } + Arc::clone(&datastore.chunk_store) + } else { + Arc::new(ChunkStore::open(name, &config.path)?) + }; - let datastore = DataStore::open_with_path(name, &path, config)?; + let datastore = DataStore::open_with_path(chunk_store, config)?; let datastore = Arc::new(datastore); map.insert(name.to_string(), datastore.clone()); @@ -81,9 +85,7 @@ impl DataStore { Ok(()) } - fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result { - let chunk_store = ChunkStore::open(store_name, path)?; - + fn open_with_path(chunk_store: Arc, config: DataStoreConfig) -> Result { let mut gc_status_path = chunk_store.base_path(); gc_status_path.push(".gc-status"); @@ -100,7 +102,7 @@ impl DataStore { }; Ok(Self { - chunk_store: Arc::new(chunk_store), + chunk_store, gc_mutex: Mutex::new(()), last_gc_status: Mutex::new(gc_status), verify_new: config.verify_new.unwrap_or(false),