[pbs-devel] [PATCH v5 proxmox 2/8] pbs api types: add option to set GC chunk cleanup atime cutoff

[Thomas Lamprecht]

Am 06.03.25 um 15:52 schrieb Christian Ebner:
> Add the `gc-atime-cutoff` option to the datastore tuning parameters.
> This allows to specify the time after which the chunks are not
> considered in use anymore if their atime has not been updated since
> then.
> 
> The default is to keep chunks within the 24h 5m timespan (given no
> active writers).
> 
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> changes since version 4:
> - drop mentioning this being conditional on gc-atime-safety-check,
>   it isn't anymore
> 
>  pbs-api-types/src/datastore.rs | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 1a4d9e2d..467417e8 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -223,6 +223,15 @@ pub enum DatastoreFSyncLevel {
>      Filesystem,
>  }
>  
> +pub const GC_ATIME_CUTOFF_SCHEMA: Schema = IntegerSchema::new(
> +    "Cutoff (in minutes) for chunk cleanup atime check in garbage collection phase 2 \
> +        (default 24h 5m)",
> +)
> +.minimum(1) // safety margin for kernel timestamp granularity, but stay within minute range
> +.maximum(2880) // 2 days
> +.default(1445)

tiny nit that could definitively get fixed up on applying or if there
is the need for a v6 due to other stuff that actually matters, otherwise
we can keep it as is:
I'd prefer writing time constants that are less common – like these here
– such, that one more easily can tell what the outcome is even without a
comment (that might get outdated), e.g. for this here:

.maximum(2 * 24 * 60)
.default(24 * 60 + 5)

Very common values like 60 m or even 600 s for 1 h are IMO normally fine
though.

> +.schema();
> +
>  #[api(
>      properties: {
>          "chunk-order": {
> @@ -237,6 +246,10 @@ pub enum DatastoreFSyncLevel {
>              default: true,
>              type: bool,
>          },
> +        "gc-atime-cutoff": {
> +            schema: GC_ATIME_CUTOFF_SCHEMA,
> +            optional: true,
> +        },
>      },
>  )]
>  #[derive(Serialize, Deserialize, Default)]
> @@ -250,6 +263,8 @@ pub struct DatastoreTuning {
>      pub sync_level: Option<DatastoreFSyncLevel>,
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub gc_atime_safety_check: Option<bool>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub gc_atime_cutoff: Option<usize>,
>  }
>  
>  pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore tuning options")