From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 1/7] datastore: gc: inline single callsite method
Date: Mon, 6 Oct 2025 12:41:45 +0200 [thread overview]
Message-ID: <20251006104151.487202-2-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251006104151.487202-1-c.ebner@proxmox.com>
This method only has a single callsite and is better split by
deduplicating common code in `ChunkStore::swipe_unused_chunks`.
Therefore, inline the method in preparation for the following
code deduplication.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
pbs-datastore/src/datastore.rs | 127 +++++++++++++--------------------
1 file changed, 49 insertions(+), 78 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 2e62590f9..c2a82b8b8 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1653,21 +1653,56 @@ impl DataStore {
let lock = self.inner.chunk_store.mutex().lock().unwrap();
for content in list_bucket_result.contents {
- if self
- .mark_chunk_for_object_key(
- &content.key,
- content.size,
- min_atime,
- oldest_writer,
- &mut delete_list,
- &mut gc_status,
- )
- .with_context(|| {
- format!("failed to mark chunk for object key {}", content.key)
- })?
- {
- chunk_count += 1;
+ let (chunk_path, digest) = match self.chunk_path_from_object_key(&content.key) {
+ Some(path) => path,
+ None => continue,
+ };
+
+ // Check local markers (created or atime updated during phase1) and
+ // keep or delete chunk based on that.
+ let atime = match std::fs::metadata(&chunk_path) {
+ Ok(stat) => stat.accessed()?,
+ Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+ // File not found, delete by setting atime to unix epoch
+ info!("Not found, mark for deletion: {}", content.key);
+ SystemTime::UNIX_EPOCH
+ }
+ Err(err) => return Err(err.into()),
+ };
+ let atime = atime.duration_since(SystemTime::UNIX_EPOCH)?.as_secs() as i64;
+
+ let bad = chunk_path
+ .as_path()
+ .extension()
+ .is_some_and(|ext| ext == "bad");
+
+ if atime < min_atime {
+ if let Some(cache) = self.cache() {
+ // ignore errors, phase 3 will retry cleanup anyways
+ let _ = cache.remove(&digest);
+ }
+ delete_list.push(content.key.clone());
+ if bad {
+ gc_status.removed_bad += 1;
+ } else {
+ gc_status.removed_chunks += 1;
+ }
+ gc_status.removed_bytes += content.size;
+ } else if atime < oldest_writer {
+ if bad {
+ gc_status.still_bad += 1;
+ } else {
+ gc_status.pending_chunks += 1;
+ }
+ gc_status.pending_bytes += content.size;
+ } else {
+ if !bad {
+ gc_status.disk_chunks += 1;
+ }
+ gc_status.disk_bytes += content.size;
}
+
+ chunk_count += 1;
}
drop(lock);
@@ -1796,70 +1831,6 @@ impl DataStore {
Ok(())
}
- // Mark the chunk marker in the local cache store for the given object key as in use
- // by updating it's atime.
- // Returns Ok(true) if the chunk was updated and Ok(false) if the object was not a chunk.
- fn mark_chunk_for_object_key(
- &self,
- object_key: &S3ObjectKey,
- size: u64,
- min_atime: i64,
- oldest_writer: i64,
- delete_list: &mut Vec<S3ObjectKey>,
- gc_status: &mut GarbageCollectionStatus,
- ) -> Result<bool, Error> {
- let (chunk_path, digest) = match self.chunk_path_from_object_key(object_key) {
- Some(path) => path,
- None => return Ok(false),
- };
-
- // Check local markers (created or atime updated during phase1) and
- // keep or delete chunk based on that.
- let atime = match std::fs::metadata(&chunk_path) {
- Ok(stat) => stat.accessed()?,
- Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
- // File not found, delete by setting atime to unix epoch
- info!("Not found, mark for deletion: {object_key}");
- SystemTime::UNIX_EPOCH
- }
- Err(err) => return Err(err.into()),
- };
- let atime = atime.duration_since(SystemTime::UNIX_EPOCH)?.as_secs() as i64;
-
- let bad = chunk_path
- .as_path()
- .extension()
- .is_some_and(|ext| ext == "bad");
-
- if atime < min_atime {
- if let Some(cache) = self.cache() {
- // ignore errors, phase 3 will retry cleanup anyways
- let _ = cache.remove(&digest);
- }
- delete_list.push(object_key.clone());
- if bad {
- gc_status.removed_bad += 1;
- } else {
- gc_status.removed_chunks += 1;
- }
- gc_status.removed_bytes += size;
- } else if atime < oldest_writer {
- if bad {
- gc_status.still_bad += 1;
- } else {
- gc_status.pending_chunks += 1;
- }
- gc_status.pending_bytes += size;
- } else {
- if !bad {
- gc_status.disk_chunks += 1;
- }
- gc_status.disk_bytes += size;
- }
-
- Ok(true)
- }
-
// Check and generate a chunk path from given object key
fn chunk_path_from_object_key(&self, object_key: &S3ObjectKey) -> Option<(PathBuf, [u8; 32])> {
// Check object is actually a chunk
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2025-10-06 10:42 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-06 10:41 [pbs-devel] [PATCH proxmox-backup 0/7] s3 store: fix issues with chunk s3 backend upload and cache eviction Christian Ebner
2025-10-06 10:41 ` Christian Ebner [this message]
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 2/7] gc: chunk store: rework atime check and gc status into common helper Christian Ebner
2025-10-06 13:14 ` Fabian Grünbichler
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 3/7] chunk store: add and use method to remove chunks Christian Ebner
2025-10-06 13:17 ` Fabian Grünbichler
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 4/7] chunk store: fix: replace evicted cache chunks instead of truncate Christian Ebner
2025-10-06 13:18 ` Fabian Grünbichler
2025-10-06 15:35 ` Christian Ebner
2025-10-06 16:14 ` Christian Ebner
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 5/7] api: chunk upload: fix race between chunk backend upload and insert Christian Ebner
2025-10-06 13:18 ` Fabian Grünbichler
2025-10-07 10:15 ` Christian Ebner
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 6/7] api: chunk upload: fix race with garbage collection for no-cache on s3 Christian Ebner
2025-10-06 13:18 ` Fabian Grünbichler
2025-10-06 10:41 ` [pbs-devel] [PATCH proxmox-backup 7/7] pull: guard chunk upload and only insert into cache after upload Christian Ebner
2025-10-06 13:18 ` Fabian Grünbichler
2025-10-06 13:18 ` [pbs-devel] [PATCH proxmox-backup 0/7] s3 store: fix issues with chunk s3 backend upload and cache eviction Fabian Grünbichler
2025-10-08 15:22 ` [pbs-devel] superseded: " Christian Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251006104151.487202-2-c.ebner@proxmox.com \
--to=c.ebner@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.