Re: [pbs-devel] [PATCH proxmox-backup v8 33/45] datastore: local chunk reader: get cached chunk from local cache store

[Lukas Wagner <l.wagner@proxmox.com>]

With my nits addresses:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>


On  2025-07-15 14:53, Christian Ebner wrote:
> Check if a chunk is contained in the local cache and if so prefer
> fetching it from the cache instead of pulling it via the S3 api. This
> improves performance and reduces number of requests to the backend.
> 
> Basic restore performance tests:
> 
> Restored a snapshot containing the linux git repository (on-disk size
> 5.069 GiB, compressed 3.718 GiB) from an AWS S3 backed datastore, with
> and without cached contents:
> non cached: 691.95 s
> all cached:  74.89 s
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 7:
> - no changes
> 
>  pbs-datastore/src/local_chunk_reader.rs | 31 +++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-datastore/src/local_chunk_reader.rs b/pbs-datastore/src/local_chunk_reader.rs
> index f5aa217ae..7ad44c4fa 100644
> --- a/pbs-datastore/src/local_chunk_reader.rs
> +++ b/pbs-datastore/src/local_chunk_reader.rs
> @@ -2,7 +2,7 @@ use std::future::Future;
>  use std::pin::Pin;
>  use std::sync::Arc;
>  
> -use anyhow::{bail, Error};
> +use anyhow::{bail, format_err, Error};
>  use http_body_util::BodyExt;
>  
>  use pbs_api_types::CryptMode;
> @@ -68,9 +68,18 @@ impl ReadChunk for LocalChunkReader {
>      fn read_raw_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
>          let chunk = match &self.backend {
>              DatastoreBackend::Filesystem => self.store.load_chunk(digest)?,
> -            DatastoreBackend::S3(s3_client) => {
> -                proxmox_async::runtime::block_on(fetch(s3_client.clone(), digest))?

better use Arc::clone here :)

> -            }
> +            DatastoreBackend::S3(s3_client) => match self.store.cache() {
> +                None => proxmox_async::runtime::block_on(fetch(s3_client.clone(), digest))?,
> +                Some(cache) => {
> +                    let mut cacher = self
> +                        .store
> +                        .cacher()?
> +                        .ok_or(format_err!("no cacher for datastore"))?;
> +                    proxmox_async::runtime::block_on(cache.access(digest, &mut cacher))?.ok_or(
> +                        format_err!("unable to access chunk with digest {}", hex::encode(digest)),
> +                    )?
> +                }
> +            },
>          };
>          self.ensure_crypt_mode(chunk.crypt_mode()?)?;
>  
> @@ -98,7 +107,19 @@ impl AsyncReadChunk for LocalChunkReader {
>                      let raw_data = tokio::fs::read(&path).await?;
>                      DataBlob::load_from_reader(&mut &raw_data[..])?
>                  }
> -                DatastoreBackend::S3(s3_client) => fetch(s3_client.clone(), digest).await?,
> +                DatastoreBackend::S3(s3_client) => match self.store.cache() {
> +                    None => fetch(s3_client.clone(), digest).await?,

same here

> +                    Some(cache) => {
> +                        let mut cacher = self
> +                            .store
> +                            .cacher()?
> +                            .ok_or(format_err!("no cacher for datastore"))?;
> +                        cache.access(digest, &mut cacher).await?.ok_or(format_err!(
> +                            "unable to access chunk with digest {}",
> +                            hex::encode(digest)
> +                        ))?
> +                    }
> +                },
>              };
>              self.ensure_crypt_mode(chunk.crypt_mode()?)?;
>  

-- 
- Lukas



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