public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: "Proxmox Backup Server development discussion"
	<pbs-devel@lists.proxmox.com>,
	"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/2] datastore: gc: rename method and adapt incorrect comments
Date: Thu, 2 Oct 2025 14:32:35 +0200	[thread overview]
Message-ID: <36e90129-6fff-42a4-bfdf-8aff36708600@proxmox.com> (raw)
In-Reply-To: <1759407371.2x17nfaxte.astroid@yuna.none>

On 10/2/25 2:18 PM, Fabian Grünbichler wrote:
> 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!

Well... it states that it checks whether the marker file marks the chunk 
as in-use (by it's presence). but yeah, I agree that it is not worded to 
be easily comprehended.

Also forgot to adapt the info message just below, so this needs to be 
included as well anyways.

I will improve wording and send this as part of the S3 upload race 
patches I'm working on.


_______________________________________________
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:33 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
2025-10-02 12:32     ` Christian Ebner [this message]
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=36e90129-6fff-42a4-bfdf-8aff36708600@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal