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 6E6611FF17A for ; Fri, 18 Jul 2025 11:23:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 06BE31820C; Fri, 18 Jul 2025 11:24:46 +0200 (CEST) Message-ID: Date: Fri, 18 Jul 2025 11:24:11 +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-20-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: 1752830649619 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.045 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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 10/45] api: backup: conditionally upload indices to s3 object store 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:20 AM, Lukas Wagner wrote: > Reviewed-by: Lukas Wagner > > Two nits inline. > > On 2025-07-15 14:52, Christian Ebner wrote: >> If the datastore is backed by an S3 compatible object store, upload >> the dynamic or fixed index files to the object store after closing >> them. The local index files are kept in the local caching datastore >> to allow for fast and efficient content lookups, avoiding expensive >> (as in monetary cost and IO latency) requests. >> >> Signed-off-by: Christian Ebner >> --- >> changes since version 7: >> - fix clippy warning and formatting >> >> src/api2/backup/environment.rs | 34 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> >> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs >> index 3d4677975..9ad13aeb3 100644 >> --- a/src/api2/backup/environment.rs >> +++ b/src/api2/backup/environment.rs >> @@ -2,6 +2,7 @@ use anyhow::{bail, format_err, Context, Error}; >> use pbs_config::BackupLockGuard; >> >> use std::collections::HashMap; >> +use std::io::Read; >> use std::sync::{Arc, Mutex}; >> use tracing::info; >> >> @@ -18,6 +19,7 @@ use pbs_datastore::dynamic_index::DynamicIndexWriter; >> use pbs_datastore::fixed_index::FixedIndexWriter; >> use pbs_datastore::{DataBlob, DataStore, DatastoreBackend}; >> use proxmox_rest_server::{formatter::*, WorkerTask}; >> +use proxmox_s3_client::S3Client; >> >> use crate::backup::VerifyWorker; >> >> @@ -479,6 +481,13 @@ impl BackupEnvironment { >> ); >> } >> >> + // For S3 backends, upload the index file to the object store after closing >> + if let DatastoreBackend::S3(s3_client) = &self.backend { >> + self.s3_upload_index(s3_client, &data.name) >> + .context("failed to upload dynamic index to s3 backend")?; >> + self.log(format!("Uploaded index file to s3 backend: {}", data.name)) >> + } >> + >> self.log_upload_stat( >> &data.name, >> &csum, >> @@ -553,6 +562,16 @@ impl BackupEnvironment { >> ); >> } >> >> + // For S3 backends, upload the index file to the object store after closing >> + if let DatastoreBackend::S3(s3_client) = &self.backend { >> + self.s3_upload_index(s3_client, &data.name) >> + .context("failed to upload fixed index to s3 backend")?; >> + self.log(format!( >> + "Uploaded fixed index file to object store: {}", >> + data.name >> + )) >> + } > > nit: the log message differs between both cases Ah yes, good catch! Specified for both cases that is either the dynamic or fixed index. > >> + >> self.log_upload_stat( >> &data.name, >> &expected_csum, >> @@ -753,6 +772,21 @@ impl BackupEnvironment { >> >> Ok(()) >> } >> + >> + fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> Result<(), Error> { >> + let object_key = >> + pbs_datastore::s3::object_key_from_path(&self.backup_dir.relative_path(), name) >> + .context("invalid index file object key")?; >> + >> + let mut full_path = self.backup_dir.full_path(); >> + full_path.push(name); >> + let mut file = std::fs::File::open(&full_path)?; >> + let mut buffer = Vec::new(); >> + file.read_to_end(&mut buffer)?; > > nit: You can use std::fs::read() to get the Vec right away :) Ah, makes this much nicer indeed, thanks for the hint! > >> + let data = hyper::body::Bytes::from(buffer); >> + proxmox_async::runtime::block_on(s3_client.upload_with_retry(object_key, data, true))?; >> + Ok(()) >> + } >> } >> >> impl RpcEnvironment for BackupEnvironment { > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel