[pbs-devel] [PATCH proxmox-backup 0/2] wait for active operations to finish before s3 refresh

[pbs-devel] [PATCH proxmox-backup 0/2] wait for active operations to finish before s3 refresh

[Christian Ebner]

Datastore contents located on the s3 backend can be refreshed on the local
datastore cache by running an s3-refresh, which will set the maintenance mode
s3-refresh to block any read/write operation and fetch the contents to a
temporary directory before moving them in place if the fetching was successful.

Setting the maintenance mode has however no effect on already active operations,
which can cause inconsistencies once the s3 refresh moves the temporary folders
in place.

Fix this by actively waiting for ongoing read/write operations to finish before
starting with the s3-refresh.

proxmox-backup:

Christian Ebner (2):
  datastore: s3 refresh: set/unset maintenance mode in api handler
  api: datastore: wait for active operations to clear before s3 refresh

 pbs-datastore/src/datastore.rs | 26 -----------------
 src/api2/admin/datastore.rs    | 51 +++++++++++++++++++++++++++++++---
 2 files changed, 47 insertions(+), 30 deletions(-)


Summary over all repositories:
  2 files changed, 47 insertions(+), 30 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pbs-devel] [PATCH proxmox-backup 1/2] datastore: s3 refresh: set/unset maintenance mode in api handler

[Christian Ebner]

Instead of setting the maintenance mode in the datastores s3 refresh
helper method, do this in the api handler directly. Since this is
now mostly an 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 be able to start
a refresh without checking for active operations.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 26 --------------------------
 src/api2/admin/datastore.rs    | 32 ++++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 127ba1c81..d5ff6e5f7 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -2208,16 +2208,6 @@ impl DataStore {
         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}")?;
 
@@ -2231,27 +2221,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 d192ee390..00110119f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2737,22 +2737,46 @@ 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> {
+    maintenance_mode(
+        &store,
+        Some(MaintenanceMode {
+            ty: MaintenanceType::S3Refresh,
+            message: None,
+        }),
+    )
+    .context("failed to set maintenance mode")?;
+
     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())?;
+
+            maintenance_mode(&store, None).context("failed to clear maintenance mode")
+        },
     )?;
 
     Ok(json!(upid))
 }
 
+// Set or clear the datastores maintenance mode by locking and updating the datastore config
+fn maintenance_mode(store: &str, 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", store)?;
+    datastore.set_maintenance_mode(maintenance_mode)?;
+    section_config.set_data(store, "datastore", &datastore)?;
+    pbs_config::datastore::save_config(&section_config)?;
+    Ok(())
+}
+
 #[sortable]
 const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
     (
-- 
2.47.3



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

[pbs-devel] [PATCH proxmox-backup 2/2] api: datastore: wait for active operations to clear before s3 refresh

[Christian Ebner]

Currently, the s3 refresh does not take into consideration already
ongoing active operations, only blocking new ones.
This will however lead to inconsistencies if there are ongoing read
or write operations. Therefore, actively wait for ongoing operatioins
to complete before running the actual refresh.
If an abort was requested while waiting, clear the maintenance mode as
well.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 00110119f..a43aad610 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -2756,7 +2756,26 @@ pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
         Some(store.clone()),
         auth_id.to_string(),
         to_stdout,
-        move |_worker| {
+        move |worker| {
+            let mut active_operations = task_tracking::get_active_operations(&store)?;
+            let mut old_status = String::new();
+            while active_operations.read + active_operations.write > 0 {
+                if worker.abort_requested() {
+                    maintenance_mode(&store, None).context("failed to clear maintenance mode")?;
+                    bail!("aborted by user");
+                }
+                let status = format!(
+                    "cannot refresh contents, {} active read and {} active write operations",
+                    active_operations.read, active_operations.write
+                );
+                if status != old_status {
+                    info!("{status}");
+                    old_status = status;
+                }
+                std::thread::sleep(std::time::Duration::from_secs(1));
+                active_operations = task_tracking::get_active_operations(&store)?;
+            }
+
             proxmox_async::runtime::block_on(datastore.s3_refresh())?;
 
             maintenance_mode(&store, None).context("failed to clear maintenance mode")
-- 
2.47.3



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