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 E8FDC1FF16F for ; Tue, 22 Jul 2025 12:11:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 05A713609D; Tue, 22 Jul 2025 12:12:27 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Tue, 22 Jul 2025 12:11:00 +0200 Message-ID: <20250722101106.526438-45-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250722101106.526438-1-c.ebner@proxmox.com> References: <20250722101106.526438-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753179087958 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: [pbs-devel] [PATCH proxmox-backup v11 40/46] datastore: conditionally upload atime marker chunk to s3 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Since commit b18eab64 ("fix #5982: garbage collection: check atime updates are honored"), the 4 MiB fixed sized, unencypted and compressed chunk containing all zeros is inserted at datastore creation if the atime safety check is enabled. If the datastore is backed by an S3 object store, chunk uploads are avoided by checking the presence of the chunks in the local cache store. Therefore, the all zero chunk will however not be uploaded since already inserted locally. Fix this by conditionally uploading the chunk before performing the atime update check for datastores backed by S3. Signed-off-by: Christian Ebner Reviewed-by: Lukas Wagner Reviewed-by: Hannes Laimer --- changes since version 10: - no changes pbs-datastore/src/chunk_store.rs | 25 ++++++++++++++++++++++--- pbs-datastore/src/datastore.rs | 20 ++++++++++---------- src/api2/config/datastore.rs | 5 ++++- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs index 95f00e8d5..3c59612bb 100644 --- a/pbs-datastore/src/chunk_store.rs +++ b/pbs-datastore/src/chunk_store.rs @@ -9,6 +9,7 @@ use tracing::{info, warn}; use pbs_api_types::{DatastoreFSyncLevel, GarbageCollectionStatus}; use proxmox_io::ReadExt; +use proxmox_s3_client::S3Client; use proxmox_sys::fs::{create_dir, create_path, file_type_from_file_stat, CreateOptions}; use proxmox_sys::process_locker::{ ProcessLockExclusiveGuard, ProcessLockSharedGuard, ProcessLocker, @@ -454,11 +455,29 @@ impl ChunkStore { /// Uses a 4 MiB fixed size, compressed but unencrypted chunk to test. The chunk is inserted in /// the chunk store if not yet present. /// Returns with error if the check could not be performed. - pub fn check_fs_atime_updates(&self, retry_on_file_changed: bool) -> Result<(), Error> { + pub fn check_fs_atime_updates( + &self, + retry_on_file_changed: bool, + s3_client: Option>, + ) -> Result<(), Error> { let (zero_chunk, digest) = DataChunkBuilder::build_zero_chunk(None, 4096 * 1024, true)?; - let (pre_existing, _) = self.insert_chunk(&zero_chunk, &digest)?; let (path, _digest) = self.chunk_path(&digest); + if let Some(ref s3_client) = s3_client { + if let Err(err) = std::fs::metadata(&path) { + if err.kind() == std::io::ErrorKind::NotFound { + let object_key = crate::s3::object_key_from_digest(&digest)?; + proxmox_async::runtime::block_on(s3_client.upload_no_replace_with_retry( + object_key, + zero_chunk.raw_data().to_vec().into(), + )) + .context("failed to upload chunk to s3 backend")?; + } + } + } + + let (pre_existing, _) = self.insert_chunk(&zero_chunk, &digest)?; + // Take into account timestamp update granularity in the kernel // Blocking the thread is fine here since this runs in a worker. std::thread::sleep(Duration::from_secs(1)); @@ -478,7 +497,7 @@ impl ChunkStore { // two metadata calls, try to check once again on changed file if metadata_before.ino() != metadata_now.ino() { if retry_on_file_changed { - return self.check_fs_atime_updates(false); + return self.check_fs_atime_updates(false, s3_client); } bail!("chunk {path:?} changed twice during access time safety check, cannot proceed."); } diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs index 1c62f0807..a051f0556 100644 --- a/pbs-datastore/src/datastore.rs +++ b/pbs-datastore/src/datastore.rs @@ -1537,10 +1537,19 @@ impl DataStore { .parse_property_string(gc_store_config.tuning.as_deref().unwrap_or(""))?, )?; + let s3_client = match self.backend()? { + DatastoreBackend::Filesystem => None, + DatastoreBackend::S3(s3_client) => { + proxmox_async::runtime::block_on(s3_client.head_bucket()) + .context("failed to reach bucket")?; + Some(s3_client) + } + }; + if tuning.gc_atime_safety_check.unwrap_or(true) { self.inner .chunk_store - .check_fs_atime_updates(true) + .check_fs_atime_updates(true, s3_client.clone()) .context("atime safety check failed")?; info!("Access time update check successful, proceeding with GC."); } else { @@ -1579,15 +1588,6 @@ impl DataStore { 1024 * 1024 }; - let s3_client = match self.backend()? { - DatastoreBackend::Filesystem => None, - DatastoreBackend::S3(s3_client) => { - proxmox_async::runtime::block_on(s3_client.head_bucket()) - .context("failed to reach bucket")?; - Some(s3_client) - } - }; - info!("Start GC phase1 (mark used chunks)"); self.mark_used_chunks( diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 82831d3a1..1cdb98396 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -1,4 +1,5 @@ use std::path::{Path, PathBuf}; +use std::sync::Arc; use ::serde::{Deserialize, Serialize}; use anyhow::{bail, format_err, Context, Error}; @@ -118,6 +119,7 @@ pub(crate) fn do_create_datastore( .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?, )?; + let mut backend_s3_client = None; if let Some(ref backend_config) = datastore.backend { let backend_config: DatastoreBackendConfig = backend_config.parse()?; match backend_config.ty.unwrap_or_default() { @@ -142,6 +144,7 @@ pub(crate) fn do_create_datastore( // Fine to block since this runs in worker task proxmox_async::runtime::block_on(s3_client.head_bucket()) .context("failed to access bucket")?; + backend_s3_client = Some(Arc::new(s3_client)); } } } @@ -185,7 +188,7 @@ pub(crate) fn do_create_datastore( if tuning.gc_atime_safety_check.unwrap_or(true) { chunk_store - .check_fs_atime_updates(true) + .check_fs_atime_updates(true, backend_s3_client) .context("access time safety check failed")?; info!("Access time update check successful."); } else { -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel