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

[Fabian Grünbichler]

On February 19, 2025 5:48 pm, Christian Ebner wrote:
> Add the `gc-wait-period` 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. This option is only considered, if the `gc-atime-check` is
> enabled, to avoid potential data loss.
> 
> The default is to keep chunks within the 24h 5m timespan (given no
> active writers). The 5m minimum was chosen to be in line with the
> already used safety offset for garbage collection.
> 
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> changes since version 1:
> - not present in previous version
> 
>  pbs-api-types/src/datastore.rs | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index a7a9db7d..7542e969 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -223,6 +223,13 @@ pub enum DatastoreFSyncLevel {
>      Filesystem,
>  }
>  
> +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?

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..

> +        .default(1445)
> +        .schema();
> +
>  #[api(
>      properties: {
>          "chunk-order": {
> @@ -235,6 +242,10 @@ pub enum DatastoreFSyncLevel {
>              default: true,
>              type: bool,
>          },
> +        "gc-wait-period": {
> +            schema: GC_WAIT_PERIOD_SCHEMA,
> +            optional: true,
> +        },
>      },
>  )]
>  #[derive(Serialize, Deserialize, Default)]
> @@ -248,6 +259,8 @@ pub struct DatastoreTuning {
>      pub sync_level: Option<DatastoreFSyncLevel>,
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub gc_atime_check: Option<bool>,
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub gc_wait_period: Option<usize>,
>  }
>  
>  pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore tuning options")
> -- 
> 2.39.5