From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id ED63A1FF165
for <inbox@lore.proxmox.com>; Thu, 8 May 2025 15:06:34 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 6FD79FECD;
Thu, 8 May 2025 15:06:51 +0200 (CEST)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu, 8 May 2025 15:05:44 +0200
Message-Id: <20250508130555.494782-11-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250508130555.494782-1-c.ebner@proxmox.com>
References: <20250508130555.494782-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.029 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [RFC v2 proxmox-backup 10/21] datastore: mark namespace
as trash instead of deleting it
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
As for backup snapshots and groups, mark the namespace as trash
instead of removing it and the contents right away, if the trash
should not be bypassed.
Actual removal of the hirarchical folder structure has to be taken
care of by the garbage collection.
In order to avoid races during removal, first mark the namespaces as
trash pending, mark the snapshots and groups as trash and only after
rename the pending marker file to the trash marker file. By this,
concurrent backups can remove the trash pending marker to avoid the
namespace being trashed.
On re-creation of a trashed namespace remove the marker file on itself
and any parent component from deepest to shallowest. As trashing a full
namespace can also set the trash pending state for recursive namespace
cleanup, remove encounters of that marker file as well to avoid the
namespace or its parent being trashed.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
pbs-datastore/src/datastore.rs | 135 +++++++++++++++++++++++++++++----
src/api2/admin/namespace.rs | 2 +-
src/server/pull.rs | 2 +-
3 files changed, 123 insertions(+), 16 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 023a6a12e..b1d81e199 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -28,7 +28,9 @@ use pbs_api_types::{
};
use pbs_config::BackupLockGuard;
-use crate::backup_info::{BackupDir, BackupGroup, BackupInfo, ListBackupFilter, OLD_LOCKING};
+use crate::backup_info::{
+ BackupDir, BackupGroup, BackupInfo, ListBackupFilter, OLD_LOCKING, TRASH_MARKER_FILENAME,
+};
use crate::chunk_store::ChunkStore;
use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
@@ -42,6 +44,8 @@ use crate::DataBlob;
static DATASTORE_MAP: LazyLock<Mutex<HashMap<String, Arc<DataStoreImpl>>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
+const TRASH_PENDING_MARKER_FILENAME: &str = ".pending";
+
/// checks if auth_id is owner, or, if owner is a token, if
/// auth_id is the user of the token
pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error> {
@@ -554,6 +558,7 @@ impl DataStore {
ns_full_path.push(ns.path());
std::fs::create_dir_all(ns_full_path)?;
+ self.untrash_namespace(&ns)?;
Ok(ns)
}
@@ -565,6 +570,34 @@ impl DataStore {
path.exists()
}
+ /// Remove the namespace and all it's parent components from the trash by removing the trash or
+ /// trash-pending marker file for each namespace level from deepest to shallowest. Missing files
+ /// are ignored.
+ pub fn untrash_namespace(&self, namespace: &BackupNamespace) -> Result<(), Error> {
+ let mut namespace = namespace.clone();
+ while namespace.depth() > 0 {
+ let mut trash_file_path = self.base_path();
+ trash_file_path.push(namespace.path());
+ let mut pending_file_path = trash_file_path.clone();
+ pending_file_path.push(TRASH_PENDING_MARKER_FILENAME);
+ if let Err(err) = std::fs::remove_file(&pending_file_path) {
+ // ignore not found, either not trashed or un-trashed by concurrent operation
+ if err.kind() != std::io::ErrorKind::NotFound {
+ bail!("failed to remove trash-pending file {trash_file_path:?}: {err}");
+ }
+ }
+ trash_file_path.push(TRASH_MARKER_FILENAME);
+ if let Err(err) = std::fs::remove_file(&trash_file_path) {
+ // ignore not found, either not trashed or un-trashed by concurrent operation
+ if err.kind() != std::io::ErrorKind::NotFound {
+ bail!("failed to remove trash file {trash_file_path:?}: {err}");
+ }
+ }
+ namespace.pop();
+ }
+ Ok(())
+ }
+
/// Remove all backup groups of a single namespace level but not the namespace itself.
///
/// Does *not* descends into child-namespaces and doesn't remoes the namespace itself either.
@@ -574,6 +607,7 @@ impl DataStore {
pub fn remove_namespace_groups(
self: &Arc<Self>,
ns: &BackupNamespace,
+ skip_trash: bool,
) -> Result<(bool, BackupGroupDeleteStats), Error> {
// FIXME: locking? The single groups/snapshots are already protected, so may not be
// necessary (depends on what we all allow to do with namespaces)
@@ -583,20 +617,22 @@ impl DataStore {
let mut stats = BackupGroupDeleteStats::default();
for group in self.iter_backup_groups(ns.to_owned())? {
- let delete_stats = group?.destroy(true)?;
+ let delete_stats = group?.destroy(skip_trash)?;
stats.add(&delete_stats);
removed_all_groups = removed_all_groups && delete_stats.all_removed();
}
- let base_file = std::fs::File::open(self.base_path())?;
- let base_fd = base_file.as_raw_fd();
- for ty in BackupType::iter() {
- let mut ty_dir = ns.path();
- ty_dir.push(ty.to_string());
- // best effort only, but we probably should log the error
- if let Err(err) = unlinkat(Some(base_fd), &ty_dir, UnlinkatFlags::RemoveDir) {
- if err != nix::errno::Errno::ENOENT {
- log::error!("failed to remove backup type {ty} in {ns} - {err}");
+ if skip_trash {
+ let base_file = std::fs::File::open(self.base_path())?;
+ let base_fd = base_file.as_raw_fd();
+ for ty in BackupType::iter() {
+ let mut ty_dir = ns.path();
+ ty_dir.push(ty.to_string());
+ // best effort only, but we probably should log the error
+ if let Err(err) = unlinkat(Some(base_fd), &ty_dir, UnlinkatFlags::RemoveDir) {
+ if err != nix::errno::Errno::ENOENT {
+ log::error!("failed to remove backup type {ty} in {ns} - {err}");
+ }
}
}
}
@@ -613,6 +649,7 @@ impl DataStore {
self: &Arc<Self>,
ns: &BackupNamespace,
delete_groups: bool,
+ skip_trash: bool,
) -> Result<(bool, BackupGroupDeleteStats), Error> {
let store = self.name();
let mut removed_all_requested = true;
@@ -620,16 +657,68 @@ impl DataStore {
if delete_groups {
log::info!("removing whole namespace recursively below {store}:/{ns}",);
for ns in self.recursive_iter_backup_ns(ns.to_owned(), NamespaceListFilter::Active)? {
- let (removed_ns_groups, delete_stats) = self.remove_namespace_groups(&ns?)?;
+ let namespace = ns?;
+
+ if !skip_trash {
+ let mut path = self.base_path();
+ path.push(namespace.path());
+ path.push(TRASH_PENDING_MARKER_FILENAME);
+ if let Err(err) = std::fs::File::create(&path) {
+ if err.kind() != std::io::ErrorKind::AlreadyExists {
+ return Err(err).context("failed to set trash pending marker file");
+ }
+ }
+ }
+
+ let (removed_ns_groups, delete_stats) =
+ self.remove_namespace_groups(&namespace, skip_trash)?;
stats.add(&delete_stats);
removed_all_requested = removed_all_requested && removed_ns_groups;
+
+ if !skip_trash && !removed_ns_groups {
+ self.untrash_namespace(&namespace)?;
+ }
}
} else {
log::info!("pruning empty namespace recursively below {store}:/{ns}");
}
- removed_all_requested =
- removed_all_requested && self.destroy_namespace_recursive(ns, delete_groups)?;
+ if skip_trash {
+ removed_all_requested =
+ removed_all_requested && self.destroy_namespace_recursive(ns, delete_groups)?;
+ return Ok((removed_all_requested, stats));
+ }
+
+ let mut children = self
+ .recursive_iter_backup_ns(ns.to_owned(), NamespaceListFilter::Active)?
+ .collect::<Result<Vec<BackupNamespace>, Error>>()?;
+
+ children.sort_by_key(|b| std::cmp::Reverse(b.depth()));
+
+ let mut all_trashed = true;
+ for ns in children.iter() {
+ let mut ns_dir = ns.path();
+ ns_dir.push("ns");
+
+ if !ns.is_root() {
+ let mut path = self.base_path();
+ path.push(ns.path());
+
+ let pending_path = path.join(TRASH_PENDING_MARKER_FILENAME);
+ path.push(TRASH_MARKER_FILENAME);
+ if let Err(err) = std::fs::rename(pending_path, path) {
+ if err.kind() == std::io::ErrorKind::NotFound {
+ all_trashed = false;
+ } else {
+ return Err(err).context("Renaming pending marker to trash marker failed");
+ }
+ }
+ }
+ }
+
+ if !all_trashed {
+ bail!("failed to prune namespace, not empty");
+ }
Ok((removed_all_requested, stats))
}
@@ -657,6 +746,24 @@ impl DataStore {
let _ = unlinkat(Some(base_fd), &ns_dir, UnlinkatFlags::RemoveDir);
if !ns.is_root() {
+ let rel_trash_path = ns.path().join(TRASH_MARKER_FILENAME);
+ if let Err(err) =
+ unlinkat(Some(base_fd), &rel_trash_path, UnlinkatFlags::NoRemoveDir)
+ {
+ if err != nix::errno::Errno::ENOENT {
+ bail!("removing the trash file '{rel_trash_path:?}' failed - {err}")
+ }
+ }
+ let rel_pending_path = ns.path().join(TRASH_PENDING_MARKER_FILENAME);
+ if let Err(err) =
+ unlinkat(Some(base_fd), &rel_pending_path, UnlinkatFlags::NoRemoveDir)
+ {
+ if err != nix::errno::Errno::ENOENT {
+ bail!(
+ "removing the trash pending file '{rel_pending_path:?}' failed - {err}"
+ )
+ }
+ }
match unlinkat(Some(base_fd), &ns.path(), UnlinkatFlags::RemoveDir) {
Ok(()) => log::debug!("removed namespace {ns}"),
Err(nix::errno::Errno::ENOENT) => {
diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
index e5463524a..a12d97883 100644
--- a/src/api2/admin/namespace.rs
+++ b/src/api2/admin/namespace.rs
@@ -166,7 +166,7 @@ pub fn delete_namespace(
let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
- let (removed_all, stats) = datastore.remove_namespace_recursive(&ns, delete_groups)?;
+ let (removed_all, stats) = datastore.remove_namespace_recursive(&ns, delete_groups, true)?;
if !removed_all {
let err_msg = if delete_groups {
if datastore.old_locking() {
diff --git a/src/server/pull.rs b/src/server/pull.rs
index d3c6fcf6a..a60ccbf10 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -729,7 +729,7 @@ fn check_and_remove_ns(params: &PullParameters, local_ns: &BackupNamespace) -> R
let (removed_all, _delete_stats) = params
.target
.store
- .remove_namespace_recursive(local_ns, true)?;
+ .remove_namespace_recursive(local_ns, true, true)?;
Ok(removed_all)
}
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel