Re: [pbs-devel] [PATCH proxmox-backup v2] GC: chunk store: fix chunk using markers cleanup

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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@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@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel