From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 15BFF1FF136 for ; Mon, 20 Apr 2026 14:29:58 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E2EED27B37; Mon, 20 Apr 2026 14:29:57 +0200 (CEST) Date: Mon, 20 Apr 2026 14:29:51 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= Subject: Re: [PATCH proxmox-backup v6 05/15] client: backup writer: fix upload stats size and rate for push sync To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260417092621.455374-1-c.ebner@proxmox.com> <20260417092621.455374-6-c.ebner@proxmox.com> In-Reply-To: <20260417092621.455374-6-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1776688031.fv5zhjkwk5.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776688110863 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: E67DMZMQYJ2ERHV45RNG52DUY7CFBNIZ X-Message-ID-Hash: E67DMZMQYJ2ERHV45RNG52DUY7CFBNIZ X-MailFrom: f.gruenbichler@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On April 17, 2026 11:26 am, 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 inflated values for the transferred size and rate. >=20 > Use the compressed chunk size instead. To get the required > information, return the more verbose `UploadStats` on > `upload_index_chunk_info` calls and use it's compressed size for the > transferred `bytes` of `SyncStats` instead. Since `UploadStats` is > now part of a pub api, increase it's scope as well. >=20 > This is then finally being used to display the upload size and > calculate the rate for the push sync job. this would make it inconsistent with the logging when doing a regular backup though: > linux.mpxar: had to backup 13.587 MiB of 13.587 MiB (compressed 2.784 MiB= ) in 2.13 s (average 6.392 MiB/s) from backup_writer.rs:482: ``` let size_dirty =3D upload_stats.size - upload_stats.size_reused; [..] let speed: HumanByte =3D ((size_dirty * 1_000_000) / (upload_stats.duration.as_micro= s() as usize)).into(); let size_dirty: HumanByte =3D size_dirty.into(); let size_compressed: HumanByte =3D upload_stats.size_compressed= .into(); info!( "{archive}: had to backup {size_dirty} of {size} (compresse= d {size_compressed}) in {:.2} s (average {speed}/s)", upload_stats.duration.as_secs_f64() ); ``` I think if we adapt this, we should adapt it to either that calculation or unify them? >=20 > Signed-off-by: Christian Ebner > --- > changes since version 5: > - no changes >=20 > pbs-client/src/backup_stats.rs | 20 ++++++++++---------- > pbs-client/src/backup_writer.rs | 4 ++-- > src/server/push.rs | 4 ++-- > 3 files changed, 14 insertions(+), 14 deletions(-) >=20 > 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 { > } > =20 > /// 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], > } > =20 > impl UploadStats { > diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writ= er.rs > index 49aff3fdd..4a4391c8b 100644 > --- a/pbs-client/src/backup_writer.rs > +++ b/pbs-client/src/backup_writer.rs > @@ -309,7 +309,7 @@ impl BackupWriter { > archive_name: &BackupArchiveName, > stream: impl Stream>, > options: UploadOptions, > - ) -> Result { > + ) -> Result { > let mut param =3D json!({ "archive-name": archive_name }); > let (prefix, archive_size) =3D options.index_type.to_prefix_and_= size(); > if let Some(size) =3D archive_size { > @@ -391,7 +391,7 @@ impl BackupWriter { > .post(&format!("{prefix}_close"), Some(param)) > .await?; > =20 > - Ok(upload_stats.to_backup_stats()) > + Ok(upload_stats) > } > =20 > pub async fn upload_stream( > diff --git a/src/server/push.rs b/src/server/push.rs > index 697b94f2f..494e0fbce 100644 > --- a/src/server/push.rs > +++ b/src/server/push.rs > @@ -1059,8 +1059,8 @@ async fn push_index( > .await?; > =20 > Ok(SyncStats { > - chunk_count: upload_stats.chunk_count as usize, > - bytes: upload_stats.size as usize, > + chunk_count: upload_stats.chunk_count, > + bytes: upload_stats.size_compressed, > elapsed: upload_stats.duration, > removed: None, > }) > --=20 > 2.47.3 >=20 >=20 >=20 >=20 >=20 >=20