Re: [RFC PATCH proxmox-backup 5/6] refactor: rename `UploadCounters` to `UploadProgress`

[Christian Ebner <c.ebner@proxmox.com>]

With the suggested rework of the logic in patch 4, this could remain as 
is I think.

On 6/10/26 8:02 PM, Shan Shaji wrote:
> Earlier, `UploadCounters` was used for accounting upload stream
> progress information. Now the `PxarArchiverProgressStats` has been added
> as an optional field inside the struct, which makes the responsibility of
> struct not only just counting the progress but also to show the the pxar
> archive progress. So, renamed `UploadCounters` to `UploadProgress`.
> 
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>   pbs-client/src/backup_stats.rs         |  4 +--
>   pbs-client/src/backup_writer.rs        | 37 +++++++++++++-------------
>   pbs-client/src/inject_reused_chunks.rs |  8 +++---
>   3 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/pbs-client/src/backup_stats.rs b/pbs-client/src/backup_stats.rs
> index aa5ae4889..2866c29a1 100644
> --- a/pbs-client/src/backup_stats.rs
> +++ b/pbs-client/src/backup_stats.rs
> @@ -43,7 +43,7 @@ impl UploadStats {
>   
>   /// Atomic counters for accounting upload stream progress information
>   #[derive(Clone)]
> -pub(crate) struct UploadCounters {
> +pub(crate) struct UploadProgress {
>       injected_chunk_count: Arc<AtomicUsize>,
>       known_chunk_count: Arc<AtomicUsize>,
>       total_chunk_count: Arc<AtomicUsize>,
> @@ -54,7 +54,7 @@ pub(crate) struct UploadCounters {
>       pub pxar_progress: Option<Arc<PxarArchiverProgressStats>>,
>   }
>   
> -impl UploadCounters {
> +impl UploadProgress {
>       /// Create and zero init new upload counters
>       pub(crate) fn new(pxar_progress: Option<Arc<PxarArchiverProgressStats>>) -> Self {
>           Self {
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index d0246689c..68379f725 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -30,7 +30,7 @@ use proxmox_time::TimeSpan;
>   
>   use crate::pxar::PxarArchiverProgressStats;
>   
> -use super::backup_stats::{BackupStats, UploadCounters, UploadStats};
> +use super::backup_stats::{BackupStats, UploadProgress, UploadStats};
>   use super::inject_reused_chunks::{InjectChunks, InjectReusedChunks, InjectedChunksInfo};
>   use super::merge_known_chunks::{MergeKnownChunks, MergedChunkInfo};
>   
> @@ -330,8 +330,8 @@ impl BackupWriter {
>               .as_u64()
>               .unwrap();
>   
> -        let mut counters = UploadCounters::new(None);
> -        let counters_readonly = counters.clone();
> +        let mut upload_progress = UploadProgress::new(None);
> +        let upload_progress_readonly = upload_progress.clone();
>   
>           let is_fixed_chunk_size = prefix == "fixed";
>   
> @@ -343,8 +343,8 @@ impl BackupWriter {
>                   match merged_chunk_info {
>                       MergedChunkInfo::New(ref chunk_info) => {
>                           let chunk_len = chunk_info.chunk_len;
> -                        let offset =
> -                            counters.add_new_chunk(chunk_len as usize, chunk_info.chunk.raw_size());
> +                        let offset = upload_progress
> +                            .add_new_chunk(chunk_len as usize, chunk_info.chunk.raw_size());
>                           let end_offset = offset as u64 + chunk_len;
>                           let mut guard = index_csum.lock().unwrap();
>                           let csum = guard.as_mut().unwrap();
> @@ -355,7 +355,7 @@ impl BackupWriter {
>                       }
>                       MergedChunkInfo::Known(ref mut known_chunk_list) => {
>                           for (chunk_len, digest) in known_chunk_list {
> -                            let offset = counters.add_known_chunk(*chunk_len as usize);
> +                            let offset = upload_progress.add_known_chunk(*chunk_len as usize);
>                               let end_offset = offset as u64 + *chunk_len;
>                               let mut guard = index_csum.lock().unwrap();
>                               let csum = guard.as_mut().unwrap();
> @@ -379,7 +379,7 @@ impl BackupWriter {
>               prefix,
>               stream,
>               index_csum_2,
> -            counters_readonly,
> +            upload_progress_readonly,
>               options.is_metadata_mode,
>           )
>           .await?;
> @@ -780,8 +780,8 @@ impl BackupWriter {
>           pxar_archive_progress_stats: Option<Arc<PxarArchiverProgressStats>>,
>           is_metadata_mode: bool,
>       ) -> impl Future<Output = Result<UploadStats, Error>> {
> -        let mut counters = UploadCounters::new(pxar_archive_progress_stats);
> -        let counters_readonly = counters.clone();
> +        let mut upload_progress = UploadProgress::new(pxar_archive_progress_stats);
> +        let upload_progress_readonly = upload_progress.clone();
>   
>           let is_fixed_chunk_size = prefix == "fixed";
>   
> @@ -789,7 +789,7 @@ impl BackupWriter {
>           let index_csum_2 = index_csum.clone();
>   
>           let stream = stream
> -            .inject_reused_chunks(injections, counters.clone())
> +            .inject_reused_chunks(injections, upload_progress.clone())
>               .and_then(move |chunk_info| match chunk_info {
>                   InjectedChunksInfo::Known(chunks) => {
>                       // account for injected chunks
> @@ -797,7 +797,7 @@ impl BackupWriter {
>                       let mut guard = index_csum.lock().unwrap();
>                       let csum = guard.as_mut().unwrap();
>                       for chunk in chunks {
> -                        let offset = counters.add_injected_chunk(&chunk) as u64;
> +                        let offset = upload_progress.add_injected_chunk(&chunk) as u64;
>                           let digest = chunk.digest();
>                           known.push((offset, digest));
>                           let end_offset = offset + chunk.size();
> @@ -819,13 +819,14 @@ impl BackupWriter {
>                       let mut known_chunks = known_chunks.lock().unwrap();
>                       let digest = *chunk_builder.digest();
>                       let (offset, res) = if known_chunks.contains(&digest) {
> -                        let offset = counters.add_known_chunk(chunk_len) as u64;
> +                        let offset = upload_progress.add_known_chunk(chunk_len) as u64;
>                           (offset, MergedChunkInfo::Known(vec![(offset, digest)]))
>                       } else {
>                           match chunk_builder.build() {
>                               Ok((chunk, digest)) => {
> -                                let offset =
> -                                    counters.add_new_chunk(chunk_len, chunk.raw_size()) as u64;
> +                                let offset = upload_progress
> +                                    .add_new_chunk(chunk_len, chunk.raw_size())
> +                                    as u64;
>                                   known_chunks.insert(digest);
>                                   (
>                                       offset,
> @@ -863,7 +864,7 @@ impl BackupWriter {
>               prefix,
>               stream,
>               index_csum_2,
> -            counters_readonly,
> +            upload_progress_readonly,
>               is_metadata_mode,
>           )
>       }
> @@ -875,7 +876,7 @@ impl BackupWriter {
>           prefix: &str,
>           stream: impl Stream<Item = Result<MergedChunkInfo, Error>>,
>           index_csum: Arc<Mutex<Option<Sha256>>>,
> -        counters: UploadCounters,
> +        upload_progress: UploadProgress,
>           is_metadata_mode: bool,
>       ) -> impl Future<Output = Result<UploadStats, Error>> {
>           let append_chunk_path = format!("{prefix}_index");
> @@ -891,7 +892,7 @@ impl BackupWriter {
>               || archive.ends_with(".pxar.didx")
>               || archive.ends_with(".ppxar.didx")
>           {
> -            let counters = counters.clone();
> +            let counters = upload_progress.clone();
>               Some(tokio::spawn(async move {
>                   loop {
>                       tokio::time::sleep(tokio::time::Duration::from_secs(60)).await;
> @@ -998,7 +999,7 @@ impl BackupWriter {
>                       handle.abort();
>                   }
>   
> -                futures::future::ok(counters.to_upload_stats(csum, start_time.elapsed()))
> +                futures::future::ok(upload_progress.to_upload_stats(csum, start_time.elapsed()))
>               })
>       }
>   
> diff --git a/pbs-client/src/inject_reused_chunks.rs b/pbs-client/src/inject_reused_chunks.rs
> index c85a28a08..314dd94ce 100644
> --- a/pbs-client/src/inject_reused_chunks.rs
> +++ b/pbs-client/src/inject_reused_chunks.rs
> @@ -7,7 +7,7 @@ use anyhow::{Error, anyhow};
>   use futures::{Stream, ready};
>   use pin_project_lite::pin_project;
>   
> -use crate::backup_stats::UploadCounters;
> +use crate::backup_stats::UploadProgress;
>   use crate::pxar::create::ReusableDynamicEntry;
>   
>   pin_project! {
> @@ -16,7 +16,7 @@ pin_project! {
>           input: S,
>           next_injection: Option<InjectChunks>,
>           injections: Option<mpsc::Receiver<InjectChunks>>,
> -        counters: UploadCounters,
> +        counters: UploadProgress,
>       }
>   }
>   
> @@ -42,7 +42,7 @@ pub trait InjectReusedChunks: Sized {
>       fn inject_reused_chunks(
>           self,
>           injections: Option<mpsc::Receiver<InjectChunks>>,
> -        counters: UploadCounters,
> +        counters: UploadProgress,
>       ) -> InjectReusedChunksQueue<Self>;
>   }
>   
> @@ -53,7 +53,7 @@ where
>       fn inject_reused_chunks(
>           self,
>           injections: Option<mpsc::Receiver<InjectChunks>>,
> -        counters: UploadCounters,
> +        counters: UploadProgress,
>       ) -> InjectReusedChunksQueue<Self> {
>           InjectReusedChunksQueue {
>               input: self,