[pbs-devel] [PATCH v5 proxmox-backup 4/5] garbage collection: generate index file list via datastore iterators
Christian Ebner
c.ebner at proxmox.com
Wed Mar 26 11:03:32 CET 2025
Instead of iterating over all index files found in the datastore in
an unstructured manner, use the datastore iterators to logically
iterate over them as other datastore operations will.
This allows to better distinguish index files in unexpected locations
from ones in their expected location, warning the user of unexpected
ones to allow to act on possible missconfigurations. Further, this
will allow to integrate marking of snapshots with missing chunks as
incomplete/corrupt more easily and helps improve cache hits when
introducing LRU caching to avoid multiple atime updates in phase 1 of
garbage collection.
This now iterates twice over the index files, as indices in
unexpected locations are still considered by generating the list of
all index files to be found in the datastore and removing regular
index files from that list, leaving unexpected ones behind.
Further, align terminology by renaming the `list_images` method to
a more fitting `list_index_files` and the variable names accordingly.
This will reduce possible confusion since throughout the codebase and
in the documentation files referencing the data chunks are referred
to as index files. The term image on the other hand is associated
with virtual machine images and other large binary data stored as
fixed-size chunks.
Basic benchmarking:
Total GC runtime shows no significatn change (average of 3 runs):
unpatched: 155.4 ± 2.6 s
patched: 155.4 ± 3.5 s
VmPeak measured via /proc/self/status before and after
`mark_used_chunks` (proxmox-backup-proxy was restarted in between
for normalization, no changes for all 3 runs):
unpatched before: 1196032 kB
unpatched after: 1196032 kB
patched before: 1196028 kB
patched after: 1196028 kB
List image shows a slight increase due to the switch to a HashSet
(average of 3 runs):
unpatched: 64.2 ± 8.4 ms
patched: 72.8 ± 3.7 ms
Description of the PBS host and datastore:
CPU: Intel Xeon E5-2620
Datastore backing storage: ZFS RAID 10 with 3 mirrors of 2x
ST16000NM001G, mirror of 2x SAMSUNG_MZ1LB1T9HALS as special
Namespaces: 45
Groups: 182
Snapshots: 3184
Index files: 6875
Deduplication factor: 44.54
Original data usage: 120.742 TiB
On-Disk usage: 2.711 TiB (2.25%)
On-Disk chunks: 1494727
Average chunk size: 1.902 MiB
Distribution of snapshots (binned by month):
2023-11 11
2023-12 16
2024-01 30
2024-02 38
2024-03 17
2024-04 37
2024-05 17
2024-06 59
2024-07 99
2024-08 96
2024-09 115
2024-10 35
2024-11 42
2024-12 37
2025-01 162
2025-02 489
2025-03 1884
Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
changes since version 4:
- extended commit message
pbs-datastore/src/datastore.rs | 104 ++++++++++++++++++++++-----------
1 file changed, 70 insertions(+), 34 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 6c3046ff6..8ce98f1b3 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -25,7 +25,7 @@ use pbs_api_types::{
MaintenanceMode, MaintenanceType, Operation, UPID,
};
-use crate::backup_info::{BackupDir, BackupGroup};
+use crate::backup_info::{BackupDir, BackupGroup, BackupInfo};
use crate::chunk_store::ChunkStore;
use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
@@ -970,10 +970,15 @@ impl DataStore {
ListGroups::new(Arc::clone(self), ns)?.collect()
}
- fn list_images(&self) -> Result<Vec<PathBuf>, Error> {
+ /// Lookup all index files to be found in the datastore without taking any logical iteration
+ /// into account.
+ /// The filesystem is walked recursevly to detect index files based on their archive type based
+ /// on the filename. This however excludes the chunks folder, hidden files and does not follow
+ /// symlinks.
+ fn list_index_files(&self) -> Result<HashSet<PathBuf>, Error> {
let base = self.base_path();
- let mut list = vec![];
+ let mut list = HashSet::new();
use walkdir::WalkDir;
@@ -1021,7 +1026,7 @@ impl DataStore {
if archive_type == ArchiveType::FixedIndex
|| archive_type == ArchiveType::DynamicIndex
{
- list.push(path);
+ list.insert(path);
}
}
}
@@ -1107,44 +1112,75 @@ impl DataStore {
status: &mut GarbageCollectionStatus,
worker: &dyn WorkerTaskContext,
) -> Result<(), Error> {
- let image_list = self.list_images()?;
- let image_count = image_list.len();
-
+ // Iterate twice over the datastore to fetch index files, even if this comes with an
+ // additional runtime cost:
+ // - First iteration to find all index files, no matter if they are in a location expected
+ // by the datastore's hierarchy
+ // - Iterate using the datastore's helpers, so the namespaces, groups and snapshots are
+ // looked up given the expected hierarchy and iterator logic
+ //
+ // By this it is assured that all index files are used, even if they would not have been
+ // seen by the regular logic and the user is informed by the garbage collection run about
+ // the detected index files not following the iterators logic.
+
+ let mut unprocessed_index_list = self.list_index_files()?;
+ let index_count = unprocessed_index_list.len();
+
+ let mut processed_index_files = 0;
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;
+ let arc_self = Arc::new(self.clone());
+ for namespace in arc_self
+ .recursive_iter_backup_ns(BackupNamespace::root())
+ .context("creating namespace iterator failed")?
+ {
+ let namespace = namespace.context("iterating namespaces failed")?;
+ for group in arc_self.iter_backup_groups(namespace)? {
+ let group = group.context("iterating backup groups failed")?;
+ let mut snapshots = group.list_backups().context("listing snapshots failed")?;
+ // Sort by snapshot timestamp to iterate over consecutive snapshots for each image.
+ BackupInfo::sort_list(&mut snapshots, true);
+ for snapshot in snapshots {
+ for file in snapshot.files {
+ worker.check_abort()?;
+ worker.fail_on_shutdown()?;
+
+ let mut path = snapshot.backup_dir.full_path();
+ path.push(file);
+
+ let index = match self.open_index_reader(&path)? {
+ Some(index) => index,
+ None => continue,
+ };
+ self.index_mark_used_chunks(index, &path, status, worker)?;
+
+ unprocessed_index_list.remove(&path);
+
+ let percentage = (processed_index_files + 1) * 100 / index_count;
+ if percentage > last_percentage {
+ info!(
+ "marked {percentage}% ({} of {index_count} index files)",
+ processed_index_files + 1,
+ );
+ last_percentage = percentage;
+ }
+ processed_index_files += 1;
}
}
}
-
- if let Some(index) = self.open_index_reader(&img)? {
- self.index_mark_used_chunks(index, &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;
- }
}
+ let strange_paths_count = unprocessed_index_list.len();
if strange_paths_count > 0 {
- info!(
- "found (and marked) {strange_paths_count} index files outside of expected directory scheme"
- );
+ warn!("found {strange_paths_count} index files outside of expected directory scheme");
+ }
+ for path in unprocessed_index_list {
+ let index = match self.open_index_reader(&path)? {
+ Some(index) => index,
+ None => continue,
+ };
+ self.index_mark_used_chunks(index, &path, status, worker)?;
+ warn!("Marked chunks for unexpected index file at '{path:?}'");
}
Ok(())
--
2.39.5
More information about the pbs-devel
mailing list