Skip to content

Commit

Permalink
[nspcc-dev#1770] node: Do not lock on shard's Close call
Browse files Browse the repository at this point in the history
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
  • Loading branch information
carpawell authored and aprasolova committed Oct 19, 2022
1 parent 61c4994 commit 6acec2b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 15 deletions.
5 changes: 1 addition & 4 deletions pkg/local_object_storage/engine/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
29 changes: 19 additions & 10 deletions pkg/local_object_storage/engine/shards.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/local_object_storage/engine/shards_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down

0 comments on commit 6acec2b

Please sign in to comment.