all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/2] datastore: gc: rename method and adapt incorrect comments
Date: Thu, 02 Oct 2025 14:18:24 +0200	[thread overview]
Message-ID: <1759407371.2x17nfaxte.astroid@yuna.none> (raw)
In-Reply-To: <20251002091142.222897-3-c.ebner@proxmox.com>

On October 2, 2025 11:11 am, Christian Ebner wrote:
> ss the helper does not really mark the chunks but rather checks
> if it should be added to the delete list.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-datastore/src/datastore.rs | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 2e62590f9..b76bb1987 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -1654,7 +1654,7 @@ impl DataStore {
>  
>                  for content in list_bucket_result.contents {
>                      if self
> -                        .mark_chunk_for_object_key(
> +                        .check_chunk_marker_for_object_key(
>                              &content.key,
>                              content.size,
>                              min_atime,
> @@ -1796,10 +1796,9 @@ 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(
> +    // Check if the chunk marker in the local cache store for the given object key marks the chunk
> +    // object as in-use or adds it to the list of objects to delete.
> +    fn check_chunk_marker_for_object_key(

this is still wrong - this method doesn't mark anything as in-use, but
it does remove it from the cache (if eligible), which will also remove
the cache marker!

so either we really make this a pure check helper (and let it return
whether the corresponding digest should be removed from the cache?) or
we find some better name ;)

>          &self,
>          object_key: &S3ObjectKey,
>          size: u64,
> -- 
> 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


  reply	other threads:[~2025-10-02 12:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-02  9:11 [pbs-devel] [PATCH proxmox-backup 0/2] fix possible deadlock for S3 garbage Christian Ebner
2025-10-02  9:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] datastore: gc: drop mutex lock before entering async context Christian Ebner
2025-10-02  9:11 ` [pbs-devel] [PATCH proxmox-backup 2/2] datastore: gc: rename method and adapt incorrect comments Christian Ebner
2025-10-02 12:18   ` Fabian Grünbichler [this message]
2025-10-02 12:32     ` Christian Ebner
2025-10-02 12:15 ` [pbs-devel] partially-applied: [PATCH proxmox-backup 0/2] fix possible deadlock for S3 garbage Fabian Grünbichler

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=1759407371.2x17nfaxte.astroid@yuna.none \
    --to=f.gruenbichler@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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal