[pbs-devel] [PATCH proxmox 2/2] pbs api types: add option to set GC chunk cleanup wait period

[Christian Ebner]

On 3/4/25 17:01, Thomas Lamprecht wrote:
> Am 03.03.25 um 13:58 schrieb Fabian Grünbichler:
>> On February 19, 2025 5:48 pm, Christian Ebner wrote:
>>> +pub const GC_WAIT_PERIOD_SCHEMA: Schema =
>>> +    IntegerSchema::new("Wait period (in minutes) for garbage collection phase 2 chunk cleanup (default 24h 5m)")
>>> +        .minimum(5)
>>> +        .maximum(1445)
>>
>> these seem a bit conservative - if we introduce the option, we could
>> also allow to reduce the grace period to 0 or to require longer grace
>> periods?
> 
> Would be fine by me to reducing the minimum to zero. And the extra 5
> minutes are "just to be sure" safety-margin, not a requirement for
> anything IIRC.

Discussed this with Fabian rather extensively today. Only reason to keep 
a small safety margin here is for small time drift in case of remote 
storages (if they use their local time for timestamps).

But this can be much lower, would opt for 1 minute to stay within the 
minute range.

Also, atime always uses the coarse resolution for timestamp updates, 
that will also not change with the multi-grained timestamp resolutions 
in https://origin.kernel.org/doc/html/v6.13/filesystems/multigrain-ts.html
So this has to be taken into account for the atime update check, and 
since setting the atime into the past will introduce other error modes 
(permissions, fs impl, ...), a short wait of a 1 second in-between must 
be used.

Also, there is no distinction to be made between filesystems mounted 
with atime and relatime, if the explicit atime update fails, the GC 
should fail.

> 
>> wait period is also a bit of a misnomer IMHO, this is not something
>> where we "wait" per se, but rather something to account for the
>> timestamp information potentially not being accurate.. not sure what a
>> better term would be, "grace period" doesn't really fit either..
>>
>> it's a kind of "safety margin", maybe something related to that? naming
>> can be quite hard..
>>
>> maybe gc_atime_safety_margin ? that also implies that setting a
>> non-default value has potential safety implications/risks your data,
>> IMHO more than "reducing a wait period" does..
> 
> Maybe something with "cutoff", like just gc_cutoff or gc_atime_cutoff,
> as a cut-off of which chunks we even consider for removal is basically
> what this is.

I would opt for gc-atime-safety-check and gc-atime-safety-margin, to 
show that they are related and their implicit function

Also, there will be no upper limit for the gc-atime-safety-margin, as 
Fabian pointed out correctly, setting this to large values might be 
desired to avoid data loss if something is off, and one notices from 
e.g. the pending removals.