[pbs-devel] applied: [PATCH v3 proxmox-backup] partial fix #5560: client: periodically show backup progress

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Christian Ebner <c.ebner@proxmox.com>
Subject: [pbs-devel] applied: [PATCH v3 proxmox-backup] partial fix #5560: client: periodically show backup progress
Date: Thu, 17 Oct 2024 17:02:53 +0200	[thread overview]
Message-ID: <3d6ae324-7aab-4547-87f4-b63f632c0b3e@proxmox.com> (raw)
In-Reply-To: <20241011093355.218198-2-c.ebner@proxmox.com>

Am 11/10/2024 um 11:33 schrieb Christian Ebner:
> Spawn a new tokio task which about every minute displays the
> cumulative progress of the backup for pxar, ppxar or img archive
> streams. Catalog and metadata archive streams are excluded from the
> output for better readability, and because the catalog upload lives
> for the whole upload time, leading to possible temporal
> misalignments in the output. The actual payload data is written via
> the other streams anyway.
> 
> Add accounting for uploaded chunks, to distinguish from chunks queued
> for upload, but not actually uploaded yet.
> 
> Example output in the backup task log:
> ```
> ...
> INFO:  processed 2.471 GiB in 1min, uploaded 2.439 GiB
> INFO:  processed 4.963 GiB in 2min, uploaded 4.929 GiB
> INFO:  processed 7.349 GiB in 3min, uploaded 7.284 GiB
> ...
> ```
> 
> This partially fixes issue 5560:
> https://bugzilla.proxmox.com/show_bug.cgi?id=5560
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Changes since version 2, thanks Thomas for comments and Gabriel for testing:
> - Clenup log output by reducing to processed bytes, time and uploaded bytes
> - Format time in human readable manner using proxmox-time's `TimeSpan`
> - Drop all now unused atomic progress counters
> - Adapted commit message accordingly
> 
> Changes since version 1, thanks Gabriel for comments and testing:
> - Abort progress output task when upload stream is finished
> - Limit output to pxar, ppxar or img archives for cleaner output
> - Adapted commit title and message
> 
>  pbs-client/src/backup_writer.rs | 74 ++++++++++++++++++++++++++++-----
>  1 file changed, 63 insertions(+), 11 deletions(-)
> 
>

applied, thanks! With some opinionated line-reduction as follow-up (opinionated
because it was OK as is too, and I could have noticed that on the previous
review, sorry...)

Did not think it through at all, but maybe it makes sense to add the uploaded
size also to the upload stats.

And FWIW, I slightly changed my mind and now think that having the reused
counter also printed might indeed be nice, but as mentioned in the v2 reply,
we can always extend this and starting out simpler is OK – if users directly
request this you get a free told-you-so coupon applicable to me directly
though ;-P

_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel