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 1084F1FF17C for ; Wed, 9 Jul 2025 09:55:17 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E6B513515A; Wed, 9 Jul 2025 09:56:00 +0200 (CEST) Message-ID: <89607187-b898-4c51-bbff-0950a30cdd3a@proxmox.com> Date: Wed, 9 Jul 2025 09:55:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: pbs-devel@lists.proxmox.com References: <20250708170114.1556057-1-c.ebner@proxmox.com> <20250708170114.1556057-20-c.ebner@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20250708170114.1556057-20-c.ebner@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.040 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [environment.rs, data.name] Subject: Re: [pbs-devel] [PATCH proxmox-backup v6 10/37] 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/8/25 19:00, 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 > --- > src/api2/backup/environment.rs | 41 ++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs > index 3d4677975..1ee923005 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 > + )) > + } > + > self.log_upload_stat( > &data.name, > &expected_csum, > @@ -753,6 +772,28 @@ impl BackupEnvironment { > > Ok(()) > } > + > + fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> Result<(), Error> { > + let mut object_key = self.backup_dir.relative_path(); > + object_key.push(name); > + let object_key = object_key > + .as_os_str() > + .to_str() > + .ok_or_else(|| format_err!("invalid file name"))?; There is a regression from v5 here: This was forgotten to refactor to the new s3 object key helper, therefore the key is now not correctly prefixed by the `.cnt` content prefix, and slipped trough testing as with the locally cached index files PBS behaves just fine. The following diff fixes the issue and will be included in a new version: diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs index 7bfd27c49..9cfc2d528 100644 --- a/src/api2/backup/environment.rs +++ b/src/api2/backup/environment.rs @@ -778,15 +777,12 @@ impl BackupEnvironment { } fn s3_upload_index(&self, s3_client: &S3Client, name: &str) -> Result<(), Error> { - let mut object_key = self.backup_dir.relative_path(); - object_key.push(name); - let object_key = object_key - .as_os_str() - .to_str() - .ok_or_else(|| format_err!("invalid file name"))?; - - let mut full_path = self.datastore.base_path(); - full_path.push(object_key); + 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)?; I'm holding off from re-sending the patch series for now in case other regressions arise or there are further comments on the code. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel