From 8ebe95747ef0714ece1621e4fb047d47654f169c Mon Sep 17 00:00:00 2001
From: Pavel Karpy <carpawell@nspcc.ru>
Date: Fri, 30 Sep 2022 17:01:48 +0300
Subject: [PATCH] [#1770] node: Do not lock on shard's `Close` call

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
---
 pkg/local_object_storage/engine/control.go    |  5 +---
 pkg/local_object_storage/engine/shards.go     | 29 ++++++++++++-------
 .../engine/shards_test.go                     |  2 +-
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/pkg/local_object_storage/engine/control.go b/pkg/local_object_storage/engine/control.go
index 00053d2f7..e1f632d82 100644
--- a/pkg/local_object_storage/engine/control.go
+++ b/pkg/local_object_storage/engine/control.go
@@ -263,10 +263,7 @@ func (e *StorageEngine) Reload(rcfg ReConfiguration) error {
 
 	e.mtx.RUnlock()
 
-	err := e.removeShards(shardsToRemove...)
-	if err != nil {
-		return fmt.Errorf("could not remove shards: %w", err)
-	}
+	e.removeShards(shardsToRemove...)
 
 	for _, newPath := range shardsToAdd {
 		sh, err := e.createShard(rcfg.shards[newPath])
diff --git a/pkg/local_object_storage/engine/shards.go b/pkg/local_object_storage/engine/shards.go
index 91a1b2af1..c54f2cc8e 100644
--- a/pkg/local_object_storage/engine/shards.go
+++ b/pkg/local_object_storage/engine/shards.go
@@ -115,22 +115,22 @@ func (e *StorageEngine) addShard(sh *shard.Shard) error {
 }
 
 // removeShards removes specified shards. Skips non-existent shards.
-// Returns any error encountered that did not allow remove the shards.
-func (e *StorageEngine) removeShards(ids ...string) error {
-	e.mtx.Lock()
-	defer e.mtx.Unlock()
+// Logs errors about shards that it could not Close after the removal.
+func (e *StorageEngine) removeShards(ids ...string) {
+	if len(ids) == 0 {
+		return
+	}
 
+	ss := make([]shardWrapper, 0, len(ids))
+
+	e.mtx.Lock()
 	for _, id := range ids {
 		sh, found := e.shards[id]
 		if !found {
 			continue
 		}
 
-		err := sh.Close()
-		if err != nil {
-			return fmt.Errorf("could not close removed shard: %w", err)
-		}
-
+		ss = append(ss, sh)
 		delete(e.shards, id)
 
 		pool, ok := e.shardPools[id]
@@ -142,8 +142,17 @@ func (e *StorageEngine) removeShards(ids ...string) error {
 		e.log.Info("shard has been removed",
 			zap.String("id", id))
 	}
+	e.mtx.Unlock()
 
-	return nil
+	for _, sh := range ss {
+		err := sh.Close()
+		if err != nil {
+			e.log.Error("could not close removed shard",
+				zap.Stringer("id", sh.ID()),
+				zap.Error(err),
+			)
+		}
+	}
 }
 
 func generateShardID() (*shard.ID, error) {
diff --git a/pkg/local_object_storage/engine/shards_test.go b/pkg/local_object_storage/engine/shards_test.go
index 1449ea9de..67a006b5a 100644
--- a/pkg/local_object_storage/engine/shards_test.go
+++ b/pkg/local_object_storage/engine/shards_test.go
@@ -32,7 +32,7 @@ func TestRemoveShard(t *testing.T) {
 
 	for id, remove := range mSh {
 		if remove {
-			require.NoError(t, e.removeShards(id))
+			e.removeShards(id)
 		}
 	}