[791] Avoid Pearl Storage clone (#792)

* [791] Fix simple places * [791] Build fix * [791] Holder heavily refactored * [791] Fix some build errors * [791] Some fixes * [791] Do not consume holder on close * [791] Revert some changes in group.rs * [791] Build fix * [791] Build fix * [791] Add initialization protection * [791] Reduce Holder cloning overhead * [791] Build fix * [791] Build fix * [791] Build fix * [791 Warn fix * [791] Update CHANGELOG.md * [791] Updates according to code review * Update CHANGELOG.md
qoollo · May 15, 2023 · 00f439e · 00f439e
1 parent ff8b27a
commit 00f439e
Show file tree

Hide file tree

Showing 5 changed files with 260 additions and 259 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,7 +7,7 @@ Bob versions changelog
 - Added mimalloc allocator for musl target (#688)
 
 #### Changed
-
+- Avoid Pearl Storage clone (#791)
 
 #### Fixed
 

diff --git a/bob-backend/src/pearl/disk_controller.rs b/bob-backend/src/pearl/disk_controller.rs
@@ -595,16 +595,10 @@ impl DiskController {
             let holders = group.holders();
             let holders = holders.read().await;
             for holder in holders.iter() {
-                let storage = holder.storage().read().await;
-                let storage = storage.storage().clone();
-                let id = holder.get_id();
+                let holder = holder.clone();
                 futures.push(async move {
-                    match storage.close().await {
-                        Ok(_) => debug!("holder {} closed", id),
-                        Err(e) => {
-                            error!("error closing holder{}: {} (disk: {:?})", id, e, self.disk)
-                        }
-                    }
+                    holder.close_storage().await;
+                    debug!("holder {} closed", holder.get_id());
                 });
             }
         }

diff --git a/bob-backend/src/pearl/group.rs b/bob-backend/src/pearl/group.rs
@@ -91,7 +91,9 @@ impl Group {
 
     pub async fn remount(&self, pp: impl Hooks) -> AnyResult<()> {
         let _reinit_lock = self.reinit_lock.write().await;
-        self.holders.write().await.clear();
+        let cleared = self.holders.write().await.clear_and_get_values();
+        close_holders(cleared.iter()).await; // Close old holders
+        std::mem::drop(cleared); // This is to guarantee, that all resources will be released before `run_under_reinit_lock` is called
         self.run_under_reinit_lock(pp).await
     }
 
@@ -223,9 +225,11 @@ impl Group {
             // holder but instead try to restart the whole disk
             if !e.is_possible_disk_disconnection() && !e.is_duplicate() && !e.is_not_ready() {
                 error!("pearl holder will restart: {:?}", e);
-                holder.try_reinit().await?;
-                holder.prepare_storage().await?;
-                debug!("backend pearl group put common storage prepared");
+                if let Err(err) = holder.try_reinit().await {
+                    warn!("Pearl backend holder reinit ended with error: {:?}", err);
+                } else {
+                    debug!("Pearl backend holder reinited");
+                }
             }
             Err(e)
         } else {
@@ -288,9 +292,11 @@ impl Group {
         let result = holder.read(key).await;
         if let Err(e) = &result {
             if !e.is_key_not_found() && !e.is_not_ready() {
-                holder.try_reinit().await?;
-                holder.prepare_storage().await?;
-                debug!("backend pearl group get common storage prepared");
+                if let Err(err) = holder.try_reinit().await {
+                    warn!("Pearl backend holder reinit ended with error: {:?}", err);
+                } else {
+                    debug!("Pearl backend holder reinited");
+                }
             }
         }
         result
@@ -400,9 +406,11 @@ impl Group {
             // holder but instead try to restart the whole disk
             if !e.is_possible_disk_disconnection() && !e.is_duplicate() && !e.is_not_ready() {
                 error!("pearl holder will restart: {:?}", e);
-                holder.try_reinit().await?;
-                holder.prepare_storage().await?;
-                debug!("backend::pearl::group::delete_common storage prepared");
+                if let Err(err) = holder.try_reinit().await {
+                    warn!("Pearl backend holder reinit ended with error: {:?}", err);
+                } else {
+                    debug!("Pearl backend holder reinited");
+                }
             }
             Err(e)
         } else {