From: Christian Ebner <c.ebner@proxmox.com>
To: "Thomas Lamprecht" <t.lamprecht@proxmox.com>,
"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] fix #5982: garbage collection: check atime updates are honored
Date: Tue, 18 Feb 2025 14:38:37 +0100 [thread overview]
Message-ID: <d661fa00-85e5-447a-971d-61d0e703a2a8@proxmox.com> (raw)
In-Reply-To: <0a345432-cb61-43d3-af9c-6561f8798574@proxmox.com>
On 2/18/25 14:31, Thomas Lamprecht wrote:
> Am 18.02.25 um 13:39 schrieb Christian Ebner:
>> On 2/18/25 12:53, Thomas Lamprecht wrote:
>>> +1; one (additional) option _might_ be to trigger suck a check on
>>> datastore creation, e.g. create the all-zero chunk and then do that
>>> test. As of now that probably would not win us much, but if we make
>>> the 24h-wait opt-in then users would be warned early enough, or we
>>> could even auto-set that option in such a case.
>>
>> Only checking the atime update check on datastore creation is not enough
>> I think, as the backing filesystem might get remounted with changed
>> mount parameters? Or do you mean to *also* check on datastore creation
>> already to early on detect issues? Although, in my testing even with
>> `noatime` the atime update seems to be honored by the way the garbage
>> collection performs the time updates (further details see below).
>
> yes, I meant doing that additionally to checking on GC.
>
>> Anyways, creating the all-zero chunk and use that for the check sounds
>> like a good optimization to me, as that allows to avoid conditional
>> checking in the phase 1 of garbage collection. However, at the cost of
>> having to make sure that it is never cleaned up by phase 2...
>
> I saw your second reply already but even without that in mind it would
> be IMO fine to only use the all-zero chunk for the on-create check, as
> I would not see it as a big problem if it then gets pruned during
> GC, if the latter would use an actually existing chunk. But no hard
> feelings here at all either way.
I think using a 4M fixed size chunk for both cases makes it even more
elegant, as one can then use the same logic for both cases, the check on
datastore create and the check on garbage collection. And to be
backwards compatible, this simply creates the zero chunk for both cases
if it does not exist, covering existing datastores already.
>> Regarding the 24 hour waiting period, as mentioned above I noted that
>> atime updates are honored even if I set the `noatime` for an ext4 or
>> `atime=off` on zfs.
>> Seems like the utimensat() bypasses this directly, as it calls into
>> vfs_utimes() [0], which sets this to be an explicit time update,
>> followed by the notify_change() [1], which then calls the setattr() of
>> the corresponding filesystem [2] via the given inode.
>> This seems to bypass the atime_needs_update() [3], only called by
>> touch_atime(). The atime_needs_update() also checks the
>> relatime_needs_update() [4].
>>
>> Although not conclusive (yet).
>>
>
> Yeah, that would support making this opt-in. FWIW, we could maybe "sell"
> this as sort of feature by not just transforming it into a boolean
> "24h-wait period <true|false>" option but rather a more generic
> "wait-period <X hours>" option that defaults to 0 hours (or maybe a
> few minutes if we want to support minute granularity). Not sure if there
> are enough (real world) use cases to warrant this, so mostly mentioned
> for the sake of completeness.
Yes, that sounds good to me!
_______________________________________________
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-02-18 13:38 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-17 13:12 [pbs-devel] [PATCH proxmox proxmox-backup 0/2] fix #5892: check atime update is honored Christian Ebner
2025-02-17 13:12 ` [pbs-devel] [PATCH proxmox 1/2] pbs api types: Add check garbage collection atime updates flag Christian Ebner
2025-02-17 13:12 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #5982: garbage collection: check atime updates are honored Christian Ebner
2025-02-17 15:36 ` Fabian Grünbichler
2025-02-17 15:57 ` Christian Ebner
2025-02-18 11:53 ` Thomas Lamprecht
2025-02-18 12:39 ` Christian Ebner
2025-02-18 13:17 ` Christian Ebner
2025-02-18 13:31 ` Thomas Lamprecht
2025-02-18 13:38 ` Christian Ebner [this message]
2025-02-19 16:54 ` [pbs-devel] [PATCH proxmox proxmox-backup 0/2] fix #5892: check atime update is honored Christian Ebner
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=d661fa00-85e5-447a-971d-61d0e703a2a8@proxmox.com \
--to=c.ebner@proxmox.com \
--cc=f.gruenbichler@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=t.lamprecht@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