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 7521D1FF191
for <inbox@lore.proxmox.com>; Fri, 21 Feb 2025 15:02:07 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 8C62534A3;
Fri, 21 Feb 2025 15:02:06 +0100 (CET)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Fri, 21 Feb 2025 15:01:10 +0100
Message-Id: <20250221140110.377328-6-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250221140110.377328-1-c.ebner@proxmox.com>
References: <20250221140110.377328-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.031 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] [PATCH proxmox-backup 5/5] fix #5331: garbage
collection: avoid multiple chunk atime updates
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>
Reduce the number of atime updates on the same chunk by logically
iterating over image index files, following the incremental backup
logic. By inserting paths for encountered images during
`list_images` using the GroupedImageList structure, the iteration
happens now for the same image filenames in the same image namespace
and group in a order based on the snapshot timestamp. For each image,
keep track of the encountered chunk digests, and remember these as
seen for the next snapshot. Chunks which have been encountered in the
previous image index, but are not present anymore are removed from
the list after each image, in order to reduce memory footprint.
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5331
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
pbs-datastore/src/datastore.rs | 70 ++++++++++++++++++++--------------
1 file changed, 42 insertions(+), 28 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index f9047820a..992812269 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -971,14 +971,14 @@ impl DataStore {
ListGroups::new(Arc::clone(self), ns)?.collect()
}
- fn list_images(&self) -> Result<Vec<PathBuf>, Error> {
+ fn list_images(&self) -> Result<GroupedImageList, Error> {
let base = self.base_path();
- let mut list = vec![];
+ let mut list = GroupedImageList::new();
use walkdir::WalkDir;
- let walker = WalkDir::new(base).into_iter();
+ let walker = WalkDir::new(&base).into_iter();
// make sure we skip .chunks (and other hidden files to keep it simple)
fn is_hidden(entry: &walkdir::DirEntry) -> bool {
@@ -1022,7 +1022,7 @@ impl DataStore {
if archive_type == ArchiveType::FixedIndex
|| archive_type == ArchiveType::DynamicIndex
{
- list.push(path);
+ list.insert(&path, &base)?;
}
}
}
@@ -1035,6 +1035,7 @@ impl DataStore {
&self,
index: I,
file_name: &Path, // only used for error reporting
+ touched_chunks: &mut TouchedChunks,
status: &mut GarbageCollectionStatus,
worker: &dyn WorkerTaskContext,
) -> Result<(), Error> {
@@ -1045,6 +1046,12 @@ impl DataStore {
worker.check_abort()?;
worker.fail_on_shutdown()?;
let digest = index.index_digest(pos).unwrap();
+
+ // Avoid multiple expensive atime updates by utimensat
+ if touched_chunks.insert(*digest) {
+ continue;
+ }
+
if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
let hex = hex::encode(digest);
warn!(
@@ -1069,6 +1076,7 @@ impl DataStore {
fn mark_used_chunks_do(
&self,
img: &Path,
+ touched_chunks: &mut TouchedChunks,
status: &mut GarbageCollectionStatus,
worker: &dyn WorkerTaskContext,
) -> Result<(), Error> {
@@ -1079,12 +1087,12 @@ impl DataStore {
let index = FixedIndexReader::new(file).map_err(|err| {
format_err!("can't read index '{}' - {err}", img.to_string_lossy())
})?;
- self.index_mark_used_chunks(index, img, status, worker)?;
+ self.index_mark_used_chunks(index, img, touched_chunks, status, worker)?;
} else if archive_type == ArchiveType::DynamicIndex {
let index = DynamicIndexReader::new(file).map_err(|err| {
format_err!("can't read index '{}' - {err}", img.to_string_lossy())
})?;
- self.index_mark_used_chunks(index, img, status, worker)?;
+ self.index_mark_used_chunks(index, img, touched_chunks, status, worker)?;
}
}
}
@@ -1099,38 +1107,44 @@ impl DataStore {
status: &mut GarbageCollectionStatus,
worker: &dyn WorkerTaskContext,
) -> Result<(), Error> {
- let image_list = self.list_images()?;
+ let mut image_list = self.list_images()?;
let image_count = image_list.len();
let mut last_percentage: usize = 0;
- let mut strange_paths_count: u64 = 0;
-
- for (i, img) in image_list.into_iter().enumerate() {
- worker.check_abort()?;
- worker.fail_on_shutdown()?;
-
- if let Some(backup_dir_path) = img.parent() {
- let backup_dir_path = backup_dir_path.strip_prefix(self.base_path())?;
- if let Some(backup_dir_str) = backup_dir_path.to_str() {
- if pbs_api_types::parse_ns_and_snapshot(backup_dir_str).is_err() {
- strange_paths_count += 1;
+ // Optimize for avoiding updates of chunks atime in same group with same
+ // image names multiple times.
+ let mut touched_chunks = TouchedChunks::new();
+ let mut processed_images = 0;
+ for (_group, images) in image_list.groups.iter_mut() {
+ for (_image, snapshots) in images.iter_mut() {
+ // Sort by snapshot timestamp to iterate over consecutive snapshots for each image.
+ snapshots.sort_by(|a, b| a.0.cmp(&b.0));
+ for (_timestamp, img) in snapshots {
+ worker.check_abort()?;
+ worker.fail_on_shutdown()?;
+
+ self.mark_used_chunks_do(img, &mut touched_chunks, status, worker)?;
+ touched_chunks.reset();
+
+ let percentage = (processed_images + 1) * 100 / image_count;
+ if percentage > last_percentage {
+ info!(
+ "marked {percentage}% ({} of {image_count} index files)",
+ processed_images + 1,
+ );
+ last_percentage = percentage;
}
+ processed_images += 1;
}
}
+ }
- self.mark_used_chunks_do(&img, status, worker)?;
-
- let percentage = (i + 1) * 100 / image_count;
- if percentage > last_percentage {
- info!(
- "marked {percentage}% ({} of {image_count} index files)",
- i + 1,
- );
- last_percentage = percentage;
- }
+ for img in &image_list.strange_path_images {
+ self.mark_used_chunks_do(img, &mut touched_chunks, status, worker)?;
}
+ let strange_paths_count = image_list.strange_path_images.len();
if strange_paths_count > 0 {
info!(
"found (and marked) {strange_paths_count} index files outside of expected directory scheme"
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel