From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 614F11FF17A for ; Fri, 18 Jul 2025 12:11:12 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5EA5D19A70; Fri, 18 Jul 2025 12:12:20 +0200 (CEST) Message-ID: <5b96811d-0aba-46aa-8064-1bab15e01c8e@proxmox.com> Date: Fri, 18 Jul 2025 12:11:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Lukas Wagner , Proxmox Backup Server development discussion References: <20250715125332.954494-1-c.ebner@proxmox.com> <20250715125332.954494-25-c.ebner@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1752833504631 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 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 Subject: Re: [pbs-devel] [PATCH proxmox-backup v8 15/45] datastore: local chunk reader: read chunks based on backend X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On 7/18/25 10:45 AM, Lukas Wagner wrote: > With the comments addressed: > > Reviewed-by: Lukas Wagner > > > On 2025-07-15 14:53, Christian Ebner wrote: >> Get and store the datastore's backend on local chunk reader >> instantiantion and fetch chunks based on the variant from either the >> filesystem or the s3 object store. >> >> By storing the backend variant, the s3 client is instantiated only >> once and reused until the local chunk reader instance is dropped. >> >> Signed-off-by: Christian Ebner >> --- >> changes since version 7: >> - no changes >> >> pbs-datastore/Cargo.toml | 1 + >> pbs-datastore/src/local_chunk_reader.rs | 38 +++++++++++++++++++++---- >> 2 files changed, 33 insertions(+), 6 deletions(-) >> >> diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml >> index 7e56dbd31..8ce930a94 100644 >> --- a/pbs-datastore/Cargo.toml >> +++ b/pbs-datastore/Cargo.toml >> @@ -13,6 +13,7 @@ crc32fast.workspace = true >> endian_trait.workspace = true >> futures.workspace = true >> hex = { workspace = true, features = [ "serde" ] } >> +http-body-util.workspace = true >> hyper.workspace = true >> libc.workspace = true >> log.workspace = true >> diff --git a/pbs-datastore/src/local_chunk_reader.rs b/pbs-datastore/src/local_chunk_reader.rs >> index 05a70c068..f5aa217ae 100644 >> --- a/pbs-datastore/src/local_chunk_reader.rs >> +++ b/pbs-datastore/src/local_chunk_reader.rs >> @@ -3,17 +3,21 @@ use std::pin::Pin; >> use std::sync::Arc; >> >> use anyhow::{bail, Error}; >> +use http_body_util::BodyExt; >> >> use pbs_api_types::CryptMode; >> use pbs_tools::crypt_config::CryptConfig; >> +use proxmox_s3_client::S3Client; >> >> use crate::data_blob::DataBlob; >> +use crate::datastore::DatastoreBackend; >> use crate::read_chunk::{AsyncReadChunk, ReadChunk}; >> use crate::DataStore; >> >> #[derive(Clone)] >> pub struct LocalChunkReader { >> store: Arc, >> + backend: DatastoreBackend, >> crypt_config: Option>, >> crypt_mode: CryptMode, >> } >> @@ -24,8 +28,11 @@ impl LocalChunkReader { >> crypt_config: Option>, >> crypt_mode: CryptMode, >> ) -> Self { >> + // TODO: Error handling! >> + let backend = store.backend().unwrap(); >> Self { >> store, >> + backend, >> crypt_config, >> crypt_mode, >> } >> @@ -47,10 +54,26 @@ impl LocalChunkReader { >> } >> } >> >> +async fn fetch(s3_client: Arc, digest: &[u8; 32]) -> Result { >> + let object_key = crate::s3::object_key_from_digest(digest)?; >> + if let Some(response) = s3_client.get_object(object_key).await? { >> + let bytes = response.content.collect().await?.to_bytes(); >> + DataBlob::from_raw(bytes.to_vec()) >> + } else { >> + bail!("no object with digest {}", hex::encode(digest)); >> + } >> +} >> + >> impl ReadChunk for LocalChunkReader { >> fn read_raw_chunk(&self, digest: &[u8; 32]) -> Result { >> - let chunk = self.store.load_chunk(digest)?; >> + 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))? > > rather use Arc::clone(&s3_client) to avoid ambiguity Addressed this ... > >> + } >> + }; >> self.ensure_crypt_mode(chunk.crypt_mode()?)?; >> + >> Ok(chunk) >> } >> >> @@ -69,11 +92,14 @@ impl AsyncReadChunk for LocalChunkReader { >> digest: &'a [u8; 32], >> ) -> Pin> + Send + 'a>> { >> Box::pin(async move { >> - let (path, _) = self.store.chunk_path(digest); >> - >> - let raw_data = tokio::fs::read(&path).await?; >> - >> - let chunk = DataBlob::load_from_reader(&mut &raw_data[..])?; >> + let chunk = match &self.backend { >> + DatastoreBackend::Filesystem => { >> + let (path, _) = self.store.chunk_path(digest); >> + 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?, > > rather use Arc::clone(&s3_client) to avoid ambiguity ... and here as well, although both without the reference > >> + }; >> self.ensure_crypt_mode(chunk.crypt_mode()?)?; >> >> Ok(chunk) > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel