From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id E58E01FF0E5 for ; Wed, 15 Jul 2026 16:06:36 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8451221436; Wed, 15 Jul 2026 16:06:36 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 15 Jul 2026 16:06:01 +0200 Message-Id: Subject: Re: [PATCH proxmox-backup 5/6] fix #7024: cli: add more information inside the backup progress logs From: "Shan Shaji" To: "Thomas Ellmenreich" , X-Mailer: aerc 0.20.0 References: <20260713081529.62859-1-s.shaji@proxmox.com> <20260713081529.62859-6-s.shaji@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784124343978 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.188 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: HR7XSG6GLK5U6I5M473KFNXUDS43JS3L X-Message-ID-Hash: HR7XSG6GLK5U6I5M473KFNXUDS43JS3L X-MailFrom: s.shaji@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: Hi Thomas, Thanks for taking the time to test the changes.=20 On Wed Jul 15, 2026 at 2:46 PM CEST, Thomas Ellmenreich wrote: > I tested the backup client in both `img` as well as `pxar` mode and the o= utput > works well. The only thing I noticed is that the progress logs during the > backup process are now specific to the image archive type. If I have > understood correctly this is because the callback is only created for the > image archive type. As discussed offlist, the callback is also getting created inside `backup_d= irectory` for the .pxar archive type. However, the logs for `.pxar` archive is not sh= own when the change-detection-mode is legacy. Thank you for finding that. I wil= l check and send a v2. =20 =20 > Not sure if that was intentional, or I may have missed something, but if = not, > it could be useful to note this in the commit message. > > This could potentially also be something that is only active if > verbose logging is enabled. I am not so sure about that, because earlier the logs are shown by default. Also, IMHO it is nice to show the logs by default. But, I am happy to change that. May be @chris or @fabian have a suggestion?=20 > As for the tests, I did the following, always using a older client, the n= ew > client and the new client with `verbose` active. > - Image archive backup of a test node > - File archive backup of the node's /etc directory > - File archive backup of 25GB of random data > - Image archive backup of the same node with the additional 25GB > I still have all of the logs in case you want to take a look at them. > > On Mon Jul 13, 2026 at 10:15 AM CEST, Shan Shaji wrote: >> previously, the progress logs only reported the processed stream size >> and uploaded size. To improve visibility additional information from >> `PxarArchiverProgressStats` and `UploadCounters` has been included in >> the logs. >> >> To access the upload statistics, a new callback field has been added to >> the UploadOptions struct that accepts UploadCounters and TimeSpan as >> parameters. Additionally, to include pxar metadata in the logs, the >> `PxarArchiverProgressStats` instance is now created earlier and shared >> between the archiver and the progress logger callback. > > [snip] > >> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_wri= ter.rs >> index 34d8db90b..8ae0cffe8 100644 >> --- a/pbs-client/src/backup_writer.rs >> +++ b/pbs-client/src/backup_writer.rs >> @@ -874,20 +877,13 @@ impl BackupWriter { >> let (upload_queue, upload_result) =3D >> Self::append_chunk_queue(h2.clone(), wid, append_chunk_path= , counters.clone()); >> =20 >> - let progress_handle =3D if archive.ends_with(".img.fidx") >> - || archive.ends_with(".pxar.didx") >> - || archive.ends_with(".ppxar.didx") >> - { >> + let progress_handle =3D if let Some(callback) =3D progress_logg= ing_callback { >> let counters =3D counters.clone(); >> Some(tokio::spawn(async move { >> loop { >> tokio::time::sleep(tokio::time::Duration::from_secs= (60)).await; >> - >> - let size =3D HumanByte::from(counters.total_stream_= len()); >> - let size_uploaded =3D HumanByte::from(counters.uplo= aded_len()); >> let elapsed =3D TimeSpan::from(start_time.elapsed()= ); >> - >> - info!("processed {size} in {elapsed}, uploaded {siz= e_uploaded}"); > > The removed logging that logged for all archive types > >> + callback(&counters, &elapsed); >> } >> })) >> } else { > > [snip] > >> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/s= rc/main.rs >> index 5c52b7917..8ce5540d5 100644 >> --- a/proxmox-backup-client/src/main.rs >> +++ b/proxmox-backup-client/src/main.rs >> @@ -1268,6 +1307,12 @@ async fn create_backup( >> (BackupSpecificationType::IMAGE, false) =3D> { >> log_file("image", &filename, target.as_ref()); >> =20 > > Current callback (below) is only active for the Image type (as seen above= ). > >> + let progress_logger =3D |counters: &UploadCounters, ela= psed: &TimeSpan| { >> + let size =3D HumanByte::from(counters.total_stream_= len()); >> + let size_uploaded =3D HumanByte::from(counters.uplo= aded_len()); >> + log::info!("processed {size} in {elapsed}, uploaded= {size_uploaded}"); >> + }; >> + >> // 0 means fifo pipe with unknown size >> let image_file_size =3D (size !=3D 0).then_some(size); >> let upload_options =3D UploadOptions { > > [snip]