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 35C671FF165
	for <inbox@lore.proxmox.com>; Thu, 22 May 2025 08:22:26 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 841522E345;
	Thu, 22 May 2025 08:22:28 +0200 (CEST)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu, 22 May 2025 08:21:40 +0200
Message-Id: <20250522062140.51956-1-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.032 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
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [proxmox.com]
Subject: [pbs-devel] [PATCH proxmox-backup] backup info: avoid additional
 stat syscall for protected check
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>

`BackupDir::is_protected` is the general helper method to check the
protected state for a snapshot. This checks for the presence of the
protected marker file, which is performed by stating the file and
requires traversing the full path.

When generating the backup list for a backup group, the snapshot
directory contents are however scanned nevertheless. Take advantage
of this by extending the regex used to filter contents by scandir to
include also the protected marker filename and set the state based on
the presence/absence, thereby avoiding the additional stat syscall
altogether.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---

This was encountered while investigating possible causes for slow GC
phase1 since PBS version 3.4 on some particular storages as reported
in the community forum [0]. While an improvement, this is very
unlikely the issue at hand.

[0] https://forum.proxmox.com/threads/166214/

 pbs-datastore/Cargo.toml         |  2 ++
 pbs-datastore/src/backup_info.rs | 29 ++++++++++++++++++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
index 7623adc28..d7fe71b34 100644
--- a/pbs-datastore/Cargo.toml
+++ b/pbs-datastore/Cargo.toml
@@ -9,6 +9,7 @@ rust-version.workspace = true
 [dependencies]
 anyhow.workspace = true
 base64.workspace = true
+const_format.workspace = true
 crc32fast.workspace = true
 endian_trait.workspace = true
 futures.workspace = true
@@ -17,6 +18,7 @@ libc.workspace = true
 log.workspace = true
 nix.workspace = true
 openssl.workspace = true
+regex.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 tokio = { workspace = true, features = [] }
diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index d4732fdd9..dbd0905c6 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -7,6 +7,7 @@ use std::sync::{Arc, LazyLock};
 use std::time::Duration;
 
 use anyhow::{bail, format_err, Context, Error};
+use const_format::concatcp;
 
 use proxmox_sys::fs::{lock_dir_noblock, lock_dir_noblock_shared, replace_file, CreateOptions};
 use proxmox_systemd::escape_unit;
@@ -21,6 +22,11 @@ use crate::manifest::{BackupManifest, MANIFEST_LOCK_NAME};
 use crate::{DataBlob, DataStore};
 
 pub const DATASTORE_LOCKS_DIR: &str = "/run/proxmox-backup/locks";
+const PROTECTED_MARKER_FILENAME: &str = ".protected";
+
+proxmox_schema::const_regex! {
+    pub BACKUP_FILES_AND_PROTECTED_REGEX = concatcp!(r"^(.*\.([fd]idx|blob)|\", PROTECTED_MARKER_FILENAME, ")$");
+}
 
 // TODO: Remove with PBS 5
 // Note: The `expect()` call here will only happen if we can neither confirm nor deny the existence
@@ -113,9 +119,26 @@ impl BackupGroup {
                 }
 
                 let backup_dir = self.backup_dir_with_rfc3339(backup_time)?;
-                let files = list_backup_files(l2_fd, backup_time)?;
+                let mut protected = false;
+                let mut files = Vec::new();
 
-                let protected = backup_dir.is_protected();
+                proxmox_sys::fs::scandir(
+                    l2_fd,
+                    backup_time,
+                    &BACKUP_FILES_AND_PROTECTED_REGEX,
+                    |_, filename, file_type| {
+                        if file_type != nix::dir::Type::File {
+                            return Ok(());
+                        }
+                        // avoids more expensive check via `BackupDir::is_protected`
+                        if filename == ".protected" {
+                            protected = true;
+                        } else {
+                            files.push(filename.to_owned());
+                        }
+                        Ok(())
+                    },
+                )?;
 
                 list.push(BackupInfo {
                     backup_dir,
@@ -457,7 +480,7 @@ impl BackupDir {
 
     pub fn protected_file(&self) -> PathBuf {
         let mut path = self.full_path();
-        path.push(".protected");
+        path.push(PROTECTED_MARKER_FILENAME);
         path
     }
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel