all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v3 4/7] datastore: s3 refresh: set/unset maintenance mode in api handler
Date: Thu, 13 Nov 2025 15:22:11 +0100	[thread overview]
Message-ID: <20251113142214.719217-5-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251113142214.719217-1-c.ebner@proxmox.com>

Instead of setting the maintenance mode in the datastores s3 refresh
helper method, do this in the api handler directly, reusing the pre-
defined helpers.

This is in preparation for fixing the s3 refresh to only be started
after still ongoing active operations complete.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- lock only after parsing auth id, ecc.
- async to sync method handle transition has been split off from this

 pbs-datastore/src/datastore.rs | 31 ++-----------------------------
 src/api2/admin/datastore.rs    | 22 ++++++++++++++++++++--
 2 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index b9d4ed2d2..031fa958b 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -2305,23 +2305,12 @@ impl DataStore {
         *OLD_LOCKING
     }
 
-    /// Set the datastore's maintenance mode to `S3Refresh`, fetch from S3 object store, clear and
-    /// replace the local cache store contents. Once finished disable the maintenance mode again.
-    /// Returns with error for other datastore backends without setting the maintenance mode.
+    /// Fetch contents from S3 object store, clear and replace the local cache store contents.
+    /// Returns with error for non-S3 datastore backends.
     pub async fn s3_refresh(self: &Arc<Self>) -> Result<(), Error> {
         match self.backend()? {
             DatastoreBackend::Filesystem => bail!("store '{}' not backed by S3", self.name()),
             DatastoreBackend::S3(s3_client) => {
-                let self_clone = Arc::clone(self);
-                tokio::task::spawn_blocking(move || {
-                    self_clone.maintenance_mode(Some(MaintenanceMode {
-                        ty: MaintenanceType::S3Refresh,
-                        message: None,
-                    }))
-                })
-                .await?
-                .context("failed to set maintenance mode")?;
-
                 let tmp_base = proxmox_sys::fs::make_tmp_dir(self.base_path(), None)
                     .context("failed to create temporary content folder in {store_base}")?;
 
@@ -2335,27 +2324,11 @@ impl DataStore {
                     let _ = std::fs::remove_dir_all(&tmp_base);
                     return Err(err);
                 }
-
-                let self_clone = Arc::clone(self);
-                tokio::task::spawn_blocking(move || self_clone.maintenance_mode(None))
-                    .await?
-                    .context("failed to clear maintenance mode")?;
             }
         }
         Ok(())
     }
 
-    // Set or clear the datastores maintenance mode by locking and updating the datastore config
-    fn maintenance_mode(&self, maintenance_mode: Option<MaintenanceMode>) -> Result<(), Error> {
-        let _lock = pbs_config::datastore::lock_config()?;
-        let (mut section_config, _digest) = pbs_config::datastore::config()?;
-        let mut datastore: DataStoreConfig = section_config.lookup("datastore", self.name())?;
-        datastore.set_maintenance_mode(maintenance_mode)?;
-        section_config.set_data(self.name(), "datastore", &datastore)?;
-        pbs_config::datastore::save_config(&section_config)?;
-        Ok(())
-    }
-
     // Fetch the contents (metadata, no chunks) of the datastore from the S3 object store to the
     // provided temporaray directory
     async fn fetch_tmp_contents(&self, tmp_base: &Path, s3_client: &S3Client) -> Result<(), Error> {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 25cf153d0..7bd7c9f72 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2678,12 +2678,30 @@ pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
+    let _lock = pbs_config::datastore::lock_config()?;
+    let (mut section_config, _digest) = pbs_config::datastore::config()?;
+    let mut datastore_config: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+    datastore_config.set_maintenance_mode(Some(MaintenanceMode {
+        ty: MaintenanceType::S3Refresh,
+        message: None,
+    }))?;
+
+    section_config.set_data(&store, "datastore", &datastore_config)?;
+    pbs_config::datastore::save_config(&section_config)?;
+    drop(_lock);
+
     let upid = WorkerTask::new_thread(
         "s3-refresh",
-        Some(store),
+        Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| proxmox_async::runtime::block_on(datastore.s3_refresh()),
+        move |_worker| {
+            proxmox_async::runtime::block_on(datastore.s3_refresh())?;
+
+            let (_lock, config) = expect_maintenance_type(&store, MaintenanceType::S3Refresh)?;
+            unset_maintenance(_lock, config).context("failed to clear maintenance mode")
+        },
     )?;
 
     Ok(json!(upid))
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


  parent reply	other threads:[~2025-11-13 14:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-13 14:22 [pbs-devel] [PATCH proxmox-backup v3 0/7] wait for active operations to finish before s3 refresh Christian Ebner
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 1/7] api: admin: drop useless option for do_unmount_device() parameter Christian Ebner
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 2/7] api: admin: rework waiting on active operations and maintenance locking Christian Ebner
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 3/7] api: admin: make s3 refresh handler sync Christian Ebner
2025-11-13 14:22 ` Christian Ebner [this message]
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 5/7] api: datastore: wait for active operations to clear before s3 refresh Christian Ebner
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 6/7] api: config: avoid unneeded backend instance on s3 store recreation Christian Ebner
2025-11-13 14:22 ` [pbs-devel] [PATCH proxmox-backup v3 7/7] api: config: guard s3-refresh on datastore recreation Christian Ebner
2025-11-14  8:48 ` [pbs-devel] applied-series: [PATCH proxmox-backup v3 0/7] wait for active operations to finish before s3 refresh Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251113142214.719217-5-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal