[#1691] blobovniczatree: Fix active blobovnicza caching

Maintain an invariant that any blobovnicza is present either
in `opened` or in `active` map. Otherwise, the logic becomes too
complicate because it is not obvious when we should close the blobovnicza.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>

pkg/local_object_storage/blobstor/blobovniczatree/blobovnicza.go

@@ -179,19 +179,26 @@ func (b *Blobovniczas) updateAndGet(p string, old *uint64) (blobovniczaWithIndex
 	defer b.activeMtx.Unlock()
 
 	// check 2nd time to find out if it blobovnicza was activated while thread was locked
-	if tryActive, ok := b.active[p]; ok && tryActive.blz == active.blz {
+	tryActive, ok := b.active[p]
+	if ok && tryActive.blz == active.blz {
 		return tryActive, nil
 	}
 
-	// remove from opened cache (active blobovnicza should always be opened)
-	b.lruMtx.Lock()
-	b.opened.Remove(p)
-	b.lruMtx.Unlock()
+	// Remove from opened cache (active blobovnicza should always be opened).
+	// Because `onEvict` callback is called in `Remove`, we need to update
+	// active map beforehand.
 	b.active[p] = active
 
+	activePath := filepath.Join(p, u64ToHexString(active.ind))
+	b.lruMtx.Lock()
+	b.opened.Remove(activePath)
+	if ok {
+		b.opened.Add(filepath.Join(p, u64ToHexString(tryActive.ind)), tryActive.blz)
+	}
+	b.lruMtx.Unlock()
+
 	b.log.Debug("blobovnicza successfully activated",
-		zap.String("path", filepath.Join(p, u64ToHexString(active.ind))),
-	)
+		zap.String("path", activePath))
 
 	return active, nil
 }

pkg/local_object_storage/blobstor/blobovniczatree/blobovnicza_test.go

@@ -9,10 +9,71 @@ import (
 	"github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor/common"
 	"github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor/internal/blobstortest"
 	"github.com/nspcc-dev/neofs-node/pkg/util/logger/test"
+	objectSDK "github.com/nspcc-dev/neofs-sdk-go/object"
 	oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
 	"github.com/stretchr/testify/require"
 )
 
+func TestOpenedAndActive(t *testing.T) {
+	rand.Seed(1024)
+
+	l := test.NewLogger(true)
+	p, err := os.MkdirTemp("", "*")
+	require.NoError(t, err)
+
+	const (
+		width  = 2
+		depth  = 1
+		dbSize = 64 * 1024
+	)
+
+	b := NewBlobovniczaTree(
+		WithLogger(l),
+		WithObjectSizeLimit(2048),
+		WithBlobovniczaShallowWidth(width),
+		WithBlobovniczaShallowDepth(depth),
+		WithRootPath(p),
+		WithOpenedCacheSize(1),
+		WithBlobovniczaSize(dbSize))
+
+	defer os.RemoveAll(p)
+
+	require.NoError(t, b.Open(false))
+	require.NoError(t, b.Init())
+
+	type pair struct {
+		obj *objectSDK.Object
+		sid []byte
+	}
+
+	objects := make([]pair, 10)
+	for i := range objects {
+		var prm common.PutPrm
+		prm.Object = blobstortest.NewObject(1024)
+		prm.Address = object.AddressOf(prm.Object)
+		prm.RawData, err = prm.Object.Marshal()
+		require.NoError(t, err)
+
+		res, err := b.Put(prm)
+		require.NoError(t, err)
+
+		objects[i].obj = prm.Object
+		objects[i].sid = res.StorageID
+	}
+	for i := range objects {
+		var prm common.GetPrm
+		prm.Address = object.AddressOf(objects[i].obj)
+		// It is important to provide StorageID because
+		// we want to open a single blobovnicza, without other
+		// unpredictable cache effects.
+		prm.StorageID = objects[i].sid
+
+		_, err := b.Get(prm)
+		require.NoError(t, err)
+	}
+	require.NoError(t, b.Close())
+}
+
 func TestBlobovniczas(t *testing.T) {
 	rand.Seed(1024)
 

pkg/local_object_storage/blobstor/blobovniczatree/control.go

@@ -26,7 +26,7 @@ func (b *Blobovniczas) Init() error {
 	}
 
 	return b.iterateLeaves(func(p string) (bool, error) {
-		blz, err := b.openBlobovniczaNoCache(p, false)
+		blz, err := b.openBlobovniczaNoCache(p)
 		if err != nil {
 			return true, err
 		}
@@ -89,36 +89,39 @@ func (b *Blobovniczas) openBlobovnicza(p string) (*blobovnicza.Blobovnicza, erro
 		return v.(*blobovnicza.Blobovnicza), nil
 	}
 
-	blz, err := b.openBlobovniczaNoCache(p, true)
+	lvlPath := filepath.Dir(p)
+	curIndex := u64FromHexString(filepath.Base(p))
+
+	b.activeMtx.RLock()
+	defer b.activeMtx.RUnlock()
+
+	active, ok := b.active[lvlPath]
+	if ok && active.ind == curIndex {
+		return active.blz, nil
+	}
+
+	b.lruMtx.Lock()
+	defer b.lruMtx.Unlock()
+
+	v, ok = b.opened.Get(p)
+	if ok {
+		return v.(*blobovnicza.Blobovnicza), nil
+	}
+
+	blz, err := b.openBlobovniczaNoCache(p)
 	if err != nil {
 		return nil, err
 	}
 
-	b.activeMtx.Lock()
-	b.lruMtx.Lock()
-
 	b.opened.Add(p, blz)
 
-	b.lruMtx.Unlock()
-	b.activeMtx.Unlock()
-
 	return blz, nil
 }
 
-func (b *Blobovniczas) openBlobovniczaNoCache(p string, tryCache bool) (*blobovnicza.Blobovnicza, error) {
+func (b *Blobovniczas) openBlobovniczaNoCache(p string) (*blobovnicza.Blobovnicza, error) {
 	b.openMtx.Lock()
 	defer b.openMtx.Unlock()
 
-	if tryCache {
-		b.lruMtx.Lock()
-		v, ok := b.opened.Get(p)
-		b.lruMtx.Unlock()
-		if ok {
-			// blobovnicza should be opened in cache
-			return v.(*blobovnicza.Blobovnicza), nil
-		}
-	}
-
 	blz := blobovnicza.New(append(b.blzOpts,
 		blobovnicza.WithReadOnly(b.readOnly),
 		blobovnicza.WithPath(filepath.Join(b.rootPath, p)),