From 1691364653ff407590a4496cca789947d0c95a68 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 21 Jul 2022 16:26:25 +0300 Subject: [PATCH] [#1559] local_object_storage: Fix tests and some data races Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/metabase/containers.go | 3 +++ pkg/local_object_storage/metabase/control.go | 3 ++- pkg/local_object_storage/metabase/delete.go | 3 +++ pkg/local_object_storage/metabase/exists.go | 3 +++ pkg/local_object_storage/metabase/get.go | 3 +++ pkg/local_object_storage/metabase/inhume.go | 3 +++ pkg/local_object_storage/metabase/list.go | 3 +++ pkg/local_object_storage/metabase/lock.go | 3 +++ pkg/local_object_storage/metabase/movable.go | 9 +++++++++ pkg/local_object_storage/metabase/put.go | 3 +++ pkg/local_object_storage/metabase/select.go | 3 +++ pkg/local_object_storage/metabase/small.go | 3 +++ pkg/local_object_storage/shard/delete_test.go | 4 ++-- pkg/local_object_storage/writecache/storage.go | 6 +++++- 14 files changed, 48 insertions(+), 4 deletions(-) diff --git a/pkg/local_object_storage/metabase/containers.go b/pkg/local_object_storage/metabase/containers.go index 7f6ac2895..da7cb0f2b 100644 --- a/pkg/local_object_storage/metabase/containers.go +++ b/pkg/local_object_storage/metabase/containers.go @@ -10,6 +10,9 @@ import ( ) func (db *DB) Containers() (list []cid.ID, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.View(func(tx *bbolt.Tx) error { list, err = db.containers(tx) diff --git a/pkg/local_object_storage/metabase/control.go b/pkg/local_object_storage/metabase/control.go index b1e072e3a..2b7f9019f 100644 --- a/pkg/local_object_storage/metabase/control.go +++ b/pkg/local_object_storage/metabase/control.go @@ -19,7 +19,8 @@ func (db *DB) Open(readOnly bool) error { db.log.Debug("created directory for Metabase", zap.String("path", db.info.Path)) if db.boltOptions == nil { - db.boltOptions = bbolt.DefaultOptions + opts := *bbolt.DefaultOptions + db.boltOptions = &opts } db.boltOptions.ReadOnly = readOnly diff --git a/pkg/local_object_storage/metabase/delete.go b/pkg/local_object_storage/metabase/delete.go index d16167075..69ee63a24 100644 --- a/pkg/local_object_storage/metabase/delete.go +++ b/pkg/local_object_storage/metabase/delete.go @@ -40,6 +40,9 @@ type referenceCounter map[string]*referenceNumber // Delete removed object records from metabase indexes. func (db *DB) Delete(prm DeletePrm) (DeleteRes, error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err := db.boltDB.Update(func(tx *bbolt.Tx) error { return db.deleteGroup(tx, prm.addrs) }) diff --git a/pkg/local_object_storage/metabase/exists.go b/pkg/local_object_storage/metabase/exists.go index cb0602e5c..5e4454d55 100644 --- a/pkg/local_object_storage/metabase/exists.go +++ b/pkg/local_object_storage/metabase/exists.go @@ -38,6 +38,9 @@ func (p ExistsRes) Exists() bool { // // Returns an error of type apistatus.ObjectAlreadyRemoved if object has been placed in graveyard. func (db *DB) Exists(prm ExistsPrm) (res ExistsRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.View(func(tx *bbolt.Tx) error { res.exists, err = db.exists(tx, prm.addr) diff --git a/pkg/local_object_storage/metabase/get.go b/pkg/local_object_storage/metabase/get.go index 5812c373d..60ca44ed9 100644 --- a/pkg/local_object_storage/metabase/get.go +++ b/pkg/local_object_storage/metabase/get.go @@ -45,6 +45,9 @@ func (r GetRes) Header() *objectSDK.Object { // Returns an error of type apistatus.ObjectNotFound if object is missing in DB. // Returns an error of type apistatus.ObjectAlreadyRemoved if object has been placed in graveyard. func (db *DB) Get(prm GetPrm) (res GetRes, err error) { + db.modeMtx.Lock() + defer db.modeMtx.Unlock() + err = db.boltDB.View(func(tx *bbolt.Tx) error { res.hdr, err = db.get(tx, prm.addr, true, prm.raw) diff --git a/pkg/local_object_storage/metabase/inhume.go b/pkg/local_object_storage/metabase/inhume.go index 1643a5759..23454528f 100644 --- a/pkg/local_object_storage/metabase/inhume.go +++ b/pkg/local_object_storage/metabase/inhume.go @@ -82,6 +82,9 @@ var ErrLockObjectRemoval = errors.New("lock object removal") // NOTE: Marks any object with GC mark (despite any prohibitions on operations // with that object) if WithForceGCMark option has been provided. func (db *DB) Inhume(prm InhumePrm) (res InhumeRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.Update(func(tx *bbolt.Tx) error { garbageBKT := tx.Bucket(garbageBucketName) diff --git a/pkg/local_object_storage/metabase/list.go b/pkg/local_object_storage/metabase/list.go index 2f4b7f295..1b2acc319 100644 --- a/pkg/local_object_storage/metabase/list.go +++ b/pkg/local_object_storage/metabase/list.go @@ -61,6 +61,9 @@ func (l ListRes) Cursor() *Cursor { // Returns ErrEndOfListing if there are no more objects to return or count // parameter set to zero. func (db *DB) ListWithCursor(prm ListPrm) (res ListRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + result := make([]oid.Address, 0, prm.count) err = db.boltDB.View(func(tx *bbolt.Tx) error { diff --git a/pkg/local_object_storage/metabase/lock.go b/pkg/local_object_storage/metabase/lock.go index 1331d62da..ab83ee59f 100644 --- a/pkg/local_object_storage/metabase/lock.go +++ b/pkg/local_object_storage/metabase/lock.go @@ -29,6 +29,9 @@ func bucketNameLockers(idCnr cid.ID) []byte { // // Locked list should be unique. Panics if it is empty. func (db *DB) Lock(cnr cid.ID, locker oid.ID, locked []oid.ID) error { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + if len(locked) == 0 { panic("empty locked list") } diff --git a/pkg/local_object_storage/metabase/movable.go b/pkg/local_object_storage/metabase/movable.go index 578c16492..42935e391 100644 --- a/pkg/local_object_storage/metabase/movable.go +++ b/pkg/local_object_storage/metabase/movable.go @@ -49,6 +49,9 @@ func (p MovableRes) AddressList() []oid.Address { // ToMoveIt marks objects to move it into another shard. This useful for // faster HRW fetching. func (db *DB) ToMoveIt(prm ToMoveItPrm) (res ToMoveItRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.Update(func(tx *bbolt.Tx) error { toMoveIt, err := tx.CreateBucketIfNotExists(toMoveItBucketName) if err != nil { @@ -63,6 +66,9 @@ func (db *DB) ToMoveIt(prm ToMoveItPrm) (res ToMoveItRes, err error) { // DoNotMove removes `MoveIt` mark from the object. func (db *DB) DoNotMove(prm DoNotMovePrm) (res DoNotMoveRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.Update(func(tx *bbolt.Tx) error { toMoveIt := tx.Bucket(toMoveItBucketName) if toMoveIt == nil { @@ -77,6 +83,9 @@ func (db *DB) DoNotMove(prm DoNotMovePrm) (res DoNotMoveRes, err error) { // Movable returns list of marked objects to move into other shard. func (db *DB) Movable(_ MovablePrm) (MovableRes, error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + var strAddrs []string err := db.boltDB.View(func(tx *bbolt.Tx) error { diff --git a/pkg/local_object_storage/metabase/put.go b/pkg/local_object_storage/metabase/put.go index 6e36d6656..5f4c204a6 100644 --- a/pkg/local_object_storage/metabase/put.go +++ b/pkg/local_object_storage/metabase/put.go @@ -54,6 +54,9 @@ var ( // // Returns an error of type apistatus.ObjectAlreadyRemoved if object has been placed in graveyard. func (db *DB) Put(prm PutPrm) (res PutRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.Batch(func(tx *bbolt.Tx) error { return db.put(tx, prm.obj, prm.id, nil) }) diff --git a/pkg/local_object_storage/metabase/select.go b/pkg/local_object_storage/metabase/select.go index cb9370836..57ea44310 100644 --- a/pkg/local_object_storage/metabase/select.go +++ b/pkg/local_object_storage/metabase/select.go @@ -56,6 +56,9 @@ func (r SelectRes) AddressList() []oid.Address { // Select returns list of addresses of objects that match search filters. func (db *DB) Select(prm SelectPrm) (res SelectRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + if blindlyProcess(prm.filters) { return res, nil } diff --git a/pkg/local_object_storage/metabase/small.go b/pkg/local_object_storage/metabase/small.go index fc73410c5..6b2280fff 100644 --- a/pkg/local_object_storage/metabase/small.go +++ b/pkg/local_object_storage/metabase/small.go @@ -34,6 +34,9 @@ func (r IsSmallRes) BlobovniczaID() *blobovnicza.ID { // shallow path which is calculated from address and therefore it is not // indexed in metabase. func (db *DB) IsSmall(prm IsSmallPrm) (res IsSmallRes, err error) { + db.modeMtx.RLock() + defer db.modeMtx.RUnlock() + err = db.boltDB.View(func(tx *bbolt.Tx) error { res.id, err = db.isSmall(tx, prm.addr) diff --git a/pkg/local_object_storage/shard/delete_test.go b/pkg/local_object_storage/shard/delete_test.go index 1fee9bcc1..2acd094b0 100644 --- a/pkg/local_object_storage/shard/delete_test.go +++ b/pkg/local_object_storage/shard/delete_test.go @@ -7,7 +7,6 @@ import ( "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/shard" apistatus "github.com/nspcc-dev/neofs-sdk-go/client/status" cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test" - oidtest "github.com/nspcc-dev/neofs-sdk-go/object/id/test" "github.com/stretchr/testify/require" ) @@ -56,7 +55,8 @@ func testShardDelete(t *testing.T, hasWriteCache bool) { }) t.Run("small object", func(t *testing.T) { - obj.SetID(oidtest.ID()) + obj := generateObjectWithCID(t, cnr) + addAttribute(obj, "foo", "bar") addPayload(obj, 1<<5) putPrm.SetObject(obj) diff --git a/pkg/local_object_storage/writecache/storage.go b/pkg/local_object_storage/writecache/storage.go index 76e8fbe68..cc7905dc1 100644 --- a/pkg/local_object_storage/writecache/storage.go +++ b/pkg/local_object_storage/writecache/storage.go @@ -60,7 +60,11 @@ func (c *cache) openStore(readOnly bool) error { DirNameLen: 1, } - c.flushed, _ = lru.New(lruKeysCount) + // Write-cache can be opened multiple times during `SetMode`. + // flushed map must not be re-created in this case. + if c.flushed == nil { + c.flushed, _ = lru.New(lruKeysCount) + } return nil }