From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2 01/12] datastore: gc: inline single callsite method
Date: Wed, 8 Oct 2025 17:21:14 +0200 [thread overview]
Message-ID: <20251008152125.849216-2-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251008152125.849216-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..c37df94b7 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);
+ 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-08 15:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-08 15:21 [pbs-devel] [PATCH proxmox-backup v2 00/12] s3 store: fix issues with chunk s3 backend upload and cache eviction Christian Ebner
2025-10-08 15:21 ` Christian Ebner [this message]
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 02/12] gc: chunk store: rework atime check and gc status into common helper Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 03/12] chunk store: add unsafe signature to cache remove method Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 04/12] local store cache: replace evicted cache chunks instead of truncate Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 05/12] local store cache: serve response fetched from s3 backend Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 06/12] local store cache: refactor fetch and insert of chunks for " Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 07/12] local store cache: rework access cache fetching and insert logic Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 08/12] local store cache: drop obsolete cacher implementation Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 09/12] chunk store: refactor method for chunk insertion Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 10/12] api: chunk upload: fix race between chunk backend upload and insert Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 11/12] api: chunk upload: fix race with garbage collection for no-cache on s3 Christian Ebner
2025-10-08 15:21 ` [pbs-devel] [PATCH proxmox-backup v2 12/12] pull: guard chunk upload and only insert into cache after upload 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=20251008152125.849216-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.