From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 909E71FF187 for ; Mon, 3 Nov 2025 15:50:41 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 11EFE2018E; Mon, 3 Nov 2025 15:51:20 +0100 (CET) Date: Mon, 03 Nov 2025 15:51:13 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20251103113120.239455-1-c.ebner@proxmox.com> <20251103113120.239455-5-c.ebner@proxmox.com> In-Reply-To: <20251103113120.239455-5-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1762175257.ofm4ecgmdi.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762181459700 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 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 04/17] api/datastore: move backup log upload by implementing datastore helper 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On November 3, 2025 12:31 pm, Christian Ebner wrote: > In an effort to decouple the api from the datastore backend, move the > backup task log upload to use a new add blob helper method of the > datastore. > > The new helper is fully sync and called on a blocking task, thereby > now also solving the previously incorrectly blocking rename_file() in > async context. > > Signed-off-by: Christian Ebner > --- > pbs-datastore/src/datastore.rs | 22 ++++++++++++++++++++++ > src/api2/admin/datastore.rs | 25 +++++++------------------ > 2 files changed, 29 insertions(+), 18 deletions(-) > > diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs > index 46600a88c..cc1267d78 100644 > --- a/pbs-datastore/src/datastore.rs > +++ b/pbs-datastore/src/datastore.rs > @@ -2453,4 +2453,26 @@ impl DataStore { > snapshot.destroy(false, &backend)?; > Ok(()) > } > + > + /// Adds the blob to the given snapshot. > + /// Requires the caller to hold the exclusive lock. > + pub fn add_blob( > + self: &Arc, > + filename: &str, > + snapshot: BackupDir, > + blob: DataBlob, should this get a backend parameter to not require instantiating a new one in contexts where lots of blobs might be added (backup env, sync job)? > + ) -> Result<(), Error> { > + if let DatastoreBackend::S3(s3_client) = self.backend()? { > + let object_key = crate::s3::object_key_from_path(&snapshot.relative_path(), filename) > + .context("invalid client log object key")?; > + let data = hyper::body::Bytes::copy_from_slice(blob.raw_data()); > + proxmox_async::runtime::block_on(s3_client.upload_replace_with_retry(object_key, data)) > + .context("failed to upload client log to s3 backend")?; > + }; > + > + let mut path = snapshot.full_path(); > + path.push(filename); > + replace_file(&path, blob.raw_data(), CreateOptions::new(), false)?; > + Ok(()) > + } the backup env also has this, and should switch to this new helper: // always verify blob/CRC at server side let blob = DataBlob::load_from_reader(&mut &data[..])?; let raw_data = blob.raw_data(); if let DatastoreBackend::S3(s3_client) = &self.backend { let object_key = pbs_datastore::s3::object_key_from_path( &self.backup_dir.relative_path(), file_name, ) .context("invalid blob object key")?; let data = hyper::body::Bytes::copy_from_slice(raw_data); proxmox_async::runtime::block_on( s3_client.upload_replace_with_retry(object_key.clone(), data), ) .context("failed to upload blob to s3 backend")?; self.log(format!("Uploaded blob to object store: {object_key}")) } replace_file(&path, raw_data, CreateOptions::new(), false)?; > } > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index 763440df9..6881b4093 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -28,9 +28,7 @@ use proxmox_router::{ > use proxmox_rrd_api_types::{RrdMode, RrdTimeframe}; > use proxmox_schema::*; > use proxmox_sortable_macro::sortable; > -use proxmox_sys::fs::{ > - file_read_firstline, file_read_optional_string, replace_file, CreateOptions, > -}; > +use proxmox_sys::fs::{file_read_firstline, file_read_optional_string, CreateOptions}; > use proxmox_time::CalendarEvent; > use proxmox_worker_task::WorkerTaskContext; > > @@ -63,7 +61,7 @@ use pbs_datastore::manifest::BackupManifest; > use pbs_datastore::prune::compute_prune_info; > use pbs_datastore::{ > check_backup_owner, ensure_datastore_is_mounted, task_tracking, BackupDir, DataStore, > - DatastoreBackend, LocalChunkReader, StoreProgress, > + LocalChunkReader, StoreProgress, > }; > use pbs_tools::json::required_string_param; > use proxmox_rest_server::{formatter, worker_is_active, WorkerTask}; > @@ -1536,20 +1534,11 @@ pub fn upload_backup_log( > // always verify blob/CRC at server side > let blob = DataBlob::load_from_reader(&mut &data[..])?; > > - if let DatastoreBackend::S3(s3_client) = datastore.backend()? { > - let object_key = pbs_datastore::s3::object_key_from_path( > - &backup_dir.relative_path(), > - file_name.as_ref(), > - ) > - .context("invalid client log object key")?; > - let data = hyper::body::Bytes::copy_from_slice(blob.raw_data()); > - s3_client > - .upload_replace_with_retry(object_key, data) > - .await > - .context("failed to upload client log to s3 backend")?; > - }; > - > - replace_file(&path, blob.raw_data(), CreateOptions::new(), false)?; > + tokio::task::spawn_blocking(move || { > + datastore.add_blob(file_name.as_ref(), backup_dir, blob) > + }) > + .await > + .map_err(|err| format_err!("{err:#?}"))??; > > // fixme: use correct formatter > Ok(formatter::JSON_FORMATTER.format_data(Value::Null, &*rpcenv)) > -- > 2.47.3 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel