[pbs-devel] [PATCH proxmox-backup v4 1/3] fix #4077: Estimated Full metric on ext4 file systems

[Thomas Lamprecht]

Am 24/08/2022 um 12:26 schrieb Daniel Tschlatscher:
> The rrd data now includes tracking the total disk usage for the unpri-
> vileged backup user. The calculation for the estimated_time_full was
> adapted to use the total for the unpriviliged user.
> 
> The unpriv_total is the sum of the used space in the file system, plus
> the available space for the unpriviliged user.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher at proxmox.com>
> Reviewed-by: Matthias Heiserer <m.heiserer at proxmox.com>
> ---
> No changes from v3
> 
>  src/api2/status.rs              | 3 ++-
>  src/bin/proxmox-backup-proxy.rs | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index 50d401c2..b918156e 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -84,7 +84,8 @@ pub async fn datastore_status(
>          let get_rrd =
>              |what: &str| extract_rrd_data(&rrd_dir, what, RRDTimeFrame::Month, RRDMode::Average);
>  
> -        let total_res = get_rrd("total")?;
> +        // Use the space for the unpriviliged user, as e.g. ext4 reserves 5% of disks for root only
> +        let total_res = get_rrd("unpriv_total")?;

But we only just started to even record that, so it's not _that_ ideal to directly switch
as the user will notice a possible big jump after the upgrade. Can we check how many data is
in there to decide which one to use, or do it more heuristically and start recording now but
switch only in a later usage (e.g., the point release after the next one).

>          let used_res = get_rrd("used")?;
>  
>          if let (
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 214bc9b1..e5c8813f 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -1273,6 +1273,8 @@ fn rrd_update_disk_stat(disk: &DiskStat, rrd_prefix: &str) {
>          rrd_update_gauge(&rrd_key, status.total as f64);
>          let rrd_key = format!("{}/used", rrd_prefix);
>          rrd_update_gauge(&rrd_key, status.used as f64);
> +        let rrd_key = format!("{}/unpriv_total", rrd_prefix);
> +        rrd_update_gauge(&rrd_key, (status.used + status.available) as f64);

what speaks against broadcasting available? I mean one got the same information
either way, but mirroring the disk stats and returning total, used, available instead
of "two" totals sounds somewhat nicer to be from an API-User POV, but no hard feelings
here, just stuck quite a bit out to me after reviewing this series finally.

>      }
>  
>      if let Some(stat) = &disk.dev {