diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs index 396b6bde..557bb196 100644 --- a/pbs-datastore/src/backup_info.rs +++ b/pbs-datastore/src/backup_info.rs @@ -232,17 +232,34 @@ impl BackupGroup { delete_stats.increment_removed_snapshots(); } - if delete_stats.all_removed() { - std::fs::remove_dir_all(&path).map_err(|err| { - format_err!("removing group directory {:?} failed - {}", path, err) - })?; + // Note: make sure the old locking mechanism isn't used as `remove_dir_all` is not safe in + // that case + if delete_stats.all_removed() && !*OLD_LOCKING { + self.remove_group_dir()?; delete_stats.increment_removed_groups(); } - let _ = std::fs::remove_file(self.lock_path()); Ok(delete_stats) } + /// Helper function, assumes that no more snapshots are present in the group. + fn remove_group_dir(&self) -> Result<(), Error> { + let owner_path = self.store.owner_path(&self.ns, &self.group); + + std::fs::remove_file(&owner_path).map_err(|err| { + format_err!("removing the owner file '{owner_path:?}' failed - {err}") + })?; + + let path = self.full_group_path(); + + std::fs::remove_dir(&path) + .map_err(|err| format_err!("removing group directory {path:?} failed - {err}"))?; + + let _ = std::fs::remove_file(self.lock_path()); + + Ok(()) + } + /// Returns the backup owner. /// /// The backup owner is the entity who first created the backup group. @@ -581,6 +598,15 @@ impl BackupDir { let _ = std::fs::remove_file(self.manifest_lock_path()); // ignore errors let _ = std::fs::remove_file(self.lock_path()); // ignore errors + let group = BackupGroup::from(self); + let _guard = group.lock().with_context(|| { + format!("while checking if group '{group:?}' is empty during snapshot destruction") + })?; + + if group.list_backups()?.is_empty() && !*OLD_LOCKING { + group.remove_group_dir()?; + } + Ok(()) } diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 1e6157c0..ae4fb7f8 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -706,7 +706,11 @@ impl DataStore { } /// Return the path of the 'owner' file. - fn owner_path(&self, ns: &BackupNamespace, group: &pbs_api_types::BackupGroup) -> PathBuf { + pub(super) fn owner_path( + &self, + ns: &BackupNamespace, + group: &pbs_api_types::BackupGroup, + ) -> PathBuf { self.group_path(ns, group).join("owner") }