tyler
/
proxmox-backup
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

backport "datastore: lookup: reuse ChunkStore on stale datastore re-open" 

Backport of commit 0bd9c87010

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 <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2022-06-02 17:59:58 +02:00
parent cb21bf7454
commit ec44c3113b
1 changed files with 9 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/backup/datastore.rs
View File

@ -52,16 +52,20 @@ impl DataStore {
let mut map = DATASTORE_MAP.lock().unwrap(); 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! // Compare Config - if changed, create new Datastore object!
if datastore.chunk_store.base == path && if datastore.chunk_store.base == path &&
datastore.verify_new == config.verify_new.unwrap_or(false) datastore.verify_new == config.verify_new.unwrap_or(false)
{ {
return Ok(datastore.clone()); 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); let datastore = Arc::new(datastore);
map.insert(name.to_string(), datastore.clone()); map.insert(name.to_string(), datastore.clone());
@ -81,9 +85,7 @@ impl DataStore {
Ok(()) Ok(())
} }
fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<Self, Error> { fn open_with_path(chunk_store: Arc<ChunkStore>, config: DataStoreConfig) -> Result<Self, Error> {
let chunk_store = ChunkStore::open(store_name, path)?;
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");
@ -100,7 +102,7 @@ impl DataStore {
}; };
Ok(Self { Ok(Self {
chunk_store: Arc::new(chunk_store), chunk_store,
gc_mutex: Mutex::new(()), gc_mutex: Mutex::new(()),
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),