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
next prev parent 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