[pbs-devel] [PATCH proxmox-backup v2] GC: chunk store: fix chunk using markers cleanup
Fabian Grünbichler
f.gruenbichler at proxmox.com
Tue Nov 25 15:18:55 CET 2025
On November 25, 2025 3:00 pm, Christian Ebner wrote:
> Since commit 9510ef1a ("GC: assure chunk exists on s3 store when
> creating missing chunk marker") chunks which are referenced by
> an index file but do not have a local marker file are marked by a
> file with the `using` extension, so they are not cleaned up during
> phase 2 if the chunk is still present on the backend.
>
> If the chunk is however not encountered, phase 3 will see the marker
> and tries to clean it up, which currently however fails because
> it is first tried to be cleaned up from the LRU cache, the filename
> being converted to the chunk digest.
>
> Therefore, clean up any using marker file encountered during phase 3
> before any regular or bad chunk, independent from the atime.
>
> Fixes: https://forum.proxmox.com/threads/176567/post-819437
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> Changes since version 1 (thanks a lot for offlist discussion Thomas):
> - Cleanup using marker chunks independent from atime cutoff
>
> pbs-datastore/src/chunk_store.rs | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index f53460664..7fe09b914 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -25,6 +25,8 @@ use crate::file_formats::{
> };
> use crate::{DataBlob, LocalDatastoreLruCache};
>
> +const USING_MARKER_FILENAME_EXT: &str = "using";
> +
> /// File system based chunk store
> pub struct ChunkStore {
> name: String, // used for error reporting
> @@ -426,6 +428,16 @@ impl ChunkStore {
> drop(lock);
> continue;
> }
> + if filename
> + .to_bytes()
> + .ends_with(USING_MARKER_FILENAME_EXT.as_bytes())
> + {
> + unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir).map_err(|err| {
> + format_err!("unlinking chunk using marker {filename:?} failed - {err}")
> + })?;
> + drop(lock);
> + continue;
> + }
this looks okay as a stop-gap, but isn't the actual problem that
.using
and
.0.bad
have the same length, so we end up taking a codepath using a weird "bad
but not bad" filename instead of skipping those markers in phase3?
in get_chunk_iterator, we skip all files that are not 64 bytes or
64+len(.0.bad) bytes long, but then set the "bad" flag based on the
extension..
and then in cond_sweep_chunk in sweep_unused_chunk, we convert non-bad
chunk filenames to digests which then fails for the "using" filenames,
because they are too long (per the error from the forum thread).
>
> chunk_count += 1;
>
> @@ -776,7 +788,7 @@ impl ChunkStore {
> /// Helper to generate marker file path for expected chunks
> fn chunk_expected_marker_path(&self, digest: &[u8; 32]) -> PathBuf {
> let (mut path, _digest_str) = self.chunk_path(digest);
> - path.set_extension("using");
> + path.set_extension(USING_MARKER_FILENAME_EXT);
> path
> }
>
> --
> 2.47.3
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
More information about the pbs-devel
mailing list