[pbs-devel] superseded: [PATCH proxmox-backup] gc: only create locally missing chunk markers if present on S3 store

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

a variant of this will be included in the next s3-race-fix patch series
version ;)

On September 17, 2025 3:41 pm, Christian Ebner wrote:
> This fixes a logic issue with the local cache marker files for all
> indexed chunks being created during phase 1 of the garbage collection,
> independent on whether the chunks might have been moved to be bad by
> a verification job.
> 
> Therefore, check if the chunk exists in the S3 object store before
> creating the marker, if no bad chunk marker is present in the local
> store cache (this can also happen after reusing a fresh local store
> cache after a new datastore setup).
> 
> Make sure to also keep the bad chunk marker files in the local store
> cache. This is achieved by marking these unconditionally if the chunk
> marker file is not found.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Encountered while investigating an unrelated issue.
> 
>  pbs-datastore/src/datastore.rs | 73 +++++++++++++++++++---------------
>  1 file changed, 41 insertions(+), 32 deletions(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 7cf020fc0..4562b0fca 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -1308,40 +1308,49 @@ impl DataStore {
>                  }
>              }
>  
> -            match s3_client {
> -                None => {
> -                    // Filesystem backend
> -                    if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
> -                        let hex = hex::encode(digest);
> -                        warn!(
> -                            "warning: unable to access non-existent chunk {hex}, required by {file_name:?}"
> -                        );
> -
> -                        // touch any corresponding .bad files to keep them around, meaning if a chunk is
> -                        // rewritten correctly they will be removed automatically, as well as if no index
> -                        // file requires the chunk anymore (won't get to this loop then)
> -                        for i in 0..=9 {
> -                            let bad_ext = format!("{}.bad", i);
> -                            let mut bad_path = PathBuf::new();
> -                            bad_path.push(self.chunk_path(digest).0);
> -                            bad_path.set_extension(bad_ext);
> -                            self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
> -                        }
> +            if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
> +                let hex = hex::encode(digest);
> +                let mut is_bad_chunk = false;
> +
> +                // touch any corresponding .bad files to keep them around, meaning if a chunk is
> +                // rewritten correctly they will be removed automatically, as well as if no index
> +                // file requires the chunk anymore (won't get to this loop then)
> +                for i in 0..=9 {
> +                    let bad_ext = format!("{}.bad", i);
> +                    let mut bad_path = PathBuf::new();
> +                    bad_path.push(self.chunk_path(digest).0);
> +                    bad_path.set_extension(bad_ext);
> +                    if self.inner.chunk_store.cond_touch_path(&bad_path, false)? {
> +                        is_bad_chunk = true;
>                      }
>                  }
> -                Some(ref _s3_client) => {
> -                    // Update atime on local cache marker files.
> -                    if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
> -                        let (chunk_path, _digest) = self.chunk_path(digest);
> -                        // Insert empty file as marker to tell GC phase2 that this is
> -                        // a chunk still in-use, so to keep in the S3 object store.
> -                        std::fs::File::options()
> -                            .write(true)
> -                            .create_new(true)
> -                            .open(&chunk_path)
> -                            .with_context(|| {
> -                                format!("failed to create marker for chunk {}", hex::encode(digest))
> -                            })?;
> +
> +                match s3_client {
> +                    None => warn!(
> +                        "warning: unable to access non-existent chunk {hex}, required by {file_name:?}"
> +                    ),
> +                    Some(ref s3_client) => {
> +                        if !is_bad_chunk {
> +                            // Might be a new empty cache with no markers present yet, so fallback to
> +                            // check the s3 object store in that case.
> +                            let object_key = crate::s3::object_key_from_digest(digest)?;
> +                            let head_object_response =
> +                                proxmox_async::runtime::block_on(s3_client.head_object(object_key))
> +                                .context("failed to check chunk presence in s3 object store")?;
> +                            if head_object_response.is_some() {
> +                                info!("Create missing marker for existing chunk {hex}");
> +                                let (chunk_path, _digest) = self.chunk_path(digest);
> +                                // Insert empty file as marker to tell GC phase2 that this is
> +                                // a chunk still in-use, so to keep in the S3 object store.
> +                                std::fs::File::options()
> +                                    .write(true)
> +                                    .create_new(true)
> +                                    .open(&chunk_path)
> +                                    .with_context(|| {
> +                                        format!("failed to create marker for chunk {hex}")
> +                                    })?;
> +                            }
> +                        }
>                      }
>                  }
>              }
> -- 
> 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