[pbs-devel] [PATCH v4 proxmox-backup 2/7] client: backup writer: fix upload stats size and rate for push sync

[Max Carrara]

On Fri Apr 4, 2025 at 3:49 PM CEST, Christian Ebner wrote:
> Currently, the logical size of the uploaded chunks is used for size
> and upload rate calculation in case of sync jobs in push direction,
> leading to incorrect values of the actual transferred size and rate.
>
> Use the compressed chunk size instead, by returning the more verbose
> `UploadStats` on `upload_index_chunk_info` calls and use it's
> compressed size for the transferred `bytes` of `SyncStats` instead,
> being finally used to display the upload size and calculate the rate
> for the push sync job.
>
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
> changes since version 3:
> - rebased onto current master
>
>  pbs-client/src/backup_stats.rs  | 20 ++++++++++----------
>  pbs-client/src/backup_writer.rs |  4 ++--
>  src/server/push.rs              |  2 +-
>  3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/pbs-client/src/backup_stats.rs b/pbs-client/src/backup_stats.rs
> index f0563a001..edf7ef3c4 100644
> --- a/pbs-client/src/backup_stats.rs
> +++ b/pbs-client/src/backup_stats.rs
> @@ -15,16 +15,16 @@ pub struct BackupStats {
>  }
>  
>  /// Extended backup run statistics and archive checksum
> -pub(crate) struct UploadStats {
> -    pub(crate) chunk_count: usize,
> -    pub(crate) chunk_reused: usize,
> -    pub(crate) chunk_injected: usize,
> -    pub(crate) size: usize,
> -    pub(crate) size_reused: usize,
> -    pub(crate) size_injected: usize,
> -    pub(crate) size_compressed: usize,
> -    pub(crate) duration: Duration,
> -    pub(crate) csum: [u8; 32],
> +pub struct UploadStats {
> +    pub chunk_count: usize,
> +    pub chunk_reused: usize,
> +    pub chunk_injected: usize,
> +    pub size: usize,
> +    pub size_reused: usize,
> +    pub size_injected: usize,
> +    pub size_compressed: usize,
> +    pub duration: Duration,
> +    pub csum: [u8; 32],
>  }
>  
>  impl UploadStats {
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index 325425069..d93b3e7e8 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -275,7 +275,7 @@ impl BackupWriter {
>          archive_name: &BackupArchiveName,
>          stream: impl Stream<Item = Result<MergedChunkInfo, Error>>,
>          options: UploadOptions,
> -    ) -> Result<BackupStats, Error> {
> +    ) -> Result<UploadStats, Error> {
>          let mut param = json!({ "archive-name": archive_name });
>          let prefix = if let Some(size) = options.fixed_size {
>              param["size"] = size.into();
> @@ -359,7 +359,7 @@ impl BackupWriter {
>              .post(&format!("{prefix}_close"), Some(param))
>              .await?;
>  
> -        Ok(upload_stats.to_backup_stats())
> +        Ok(upload_stats)
>      }
>  
>      pub async fn upload_stream(
> diff --git a/src/server/push.rs b/src/server/push.rs
> index 0db3dff30..f00a6b1e0 100644
> --- a/src/server/push.rs
> +++ b/src/server/push.rs
> @@ -1016,7 +1016,7 @@ async fn push_index(
>  
>      Ok(SyncStats {
>          chunk_count: upload_stats.chunk_count as usize,
> -        bytes: upload_stats.size as usize,
> +        bytes: upload_stats.size_compressed as usize,

The `as usize` cast here is unnecessary, as `size_compressed` is already
`usize`. Same also goes for `chunk_count`. Can perhaps be fixed in a
follow-up, though. Tiny thing.

>          elapsed: upload_stats.duration,
>          removed: None,
>      })