public inbox for pbs-devel@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 v2 5/6] datastore: s3 refresh: set/unset maintenance mode in api handler
Date: Wed, 12 Nov 2025 17:36:23 +0100	[thread overview]
Message-ID: <20251112163624.691139-6-c.ebner@proxmox.com> (raw)
In-Reply-To: <20251112163624.691139-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. Since this is now mostly a sync task, adapt the api
handler to be a sync function and run the task on a dedicated thread.

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:
- reuse already present maintenance mode helpers from unmount operation

 pbs-datastore/src/datastore.rs | 31 ++-----------------------------
 src/api2/admin/datastore.rs    | 26 ++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 33 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 8d58b5059..91189d7ae 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2693,17 +2693,35 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
     },
 )]
 /// Refresh datastore contents from S3 to local cache store.
-pub async fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    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 datastore = DataStore::lookup_datastore(&store, Some(Operation::Lookup))?;
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-    let upid = WorkerTask::spawn(
+    let upid = WorkerTask::new_thread(
         "s3-refresh",
-        Some(store),
+        Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| async move { datastore.s3_refresh().await },
+        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-12 16:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-12 16:36 [pbs-devel] [PATCH proxmox-backup v2 0/6] wait for active operations to finish before s3 refresh Christian Ebner
2025-11-12 16:36 ` [pbs-devel] [PATCH proxmox-backup v2 1/6] api: datastore: fix typo in helper function name Christian Ebner
2025-11-12 16:36 ` [pbs-devel] [PATCH proxmox-backup v2 2/6] api: admin: make expected maintenance type helper generic over type Christian Ebner
2025-11-12 16:36 ` [pbs-devel] [PATCH proxmox-backup v2 3/6] api: admin: factor out busy waiting on active operations Christian Ebner
2025-11-13  8:15   ` Fabian Grünbichler
2025-11-13  8:38     ` Christian Ebner
2025-11-12 16:36 ` [pbs-devel] [PATCH proxmox-backup v2 4/6] api: admin: factor out locking and maintenance mode clearing Christian Ebner
2025-11-13  8:18   ` Fabian Grünbichler
2025-11-13  8:43     ` Christian Ebner
2025-11-13  9:08       ` Fabian Grünbichler
2025-11-13  9:11         ` Christian Ebner
2025-11-12 16:36 ` Christian Ebner [this message]
2025-11-12 16:36 ` [pbs-devel] [PATCH proxmox-backup v2 6/6] api: datastore: wait for active operations to clear before s3 refresh Christian Ebner
2025-11-13  8:15   ` Fabian Grünbichler
2025-11-13  9:03     ` Christian Ebner
2025-11-13  8:20 ` [pbs-devel] partially-applied: [PATCH proxmox-backup v2 0/6] wait for active operations to finish " Fabian Grünbichler
2025-11-13 14:23 ` [pbs-devel] superseded: " Christian Ebner

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=20251112163624.691139-6-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal