From 2ee374baab850dbda0fc66e34d30e22ccb8a04be Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Fri, 22 Nov 2024 10:52:52 +0300 Subject: [PATCH] shard: pass meta problem as error, drop HasMeta() from *Res If we have no error and got a result -- no one cares about HasMeta(). If we've got an error then currently some codes cares about meta status, but it can be passed as an additional error as well. Related to #3024, in reality meta should be internal to the shard, but this change at least avoids excessive booleans. Signed-off-by: Roman Khimov --- pkg/local_object_storage/engine/get.go | 20 +++++++-------- pkg/local_object_storage/engine/range.go | 4 +-- pkg/local_object_storage/shard/get.go | 30 ++++++++++++---------- pkg/local_object_storage/shard/get_test.go | 5 +--- pkg/local_object_storage/shard/range.go | 16 +++++------- 5 files changed, 36 insertions(+), 39 deletions(-) diff --git a/pkg/local_object_storage/engine/get.go b/pkg/local_object_storage/engine/get.go index ed4ba2249d..1753918011 100644 --- a/pkg/local_object_storage/engine/get.go +++ b/pkg/local_object_storage/engine/get.go @@ -40,19 +40,19 @@ func (e *StorageEngine) Get(addr oid.Address) (*objectSDK.Object, error) { ) sp.SetAddress(addr) - err = e.get(addr, func(s *shard.Shard, ignoreMetadata bool) (bool, error) { + err = e.get(addr, func(s *shard.Shard, ignoreMetadata bool) error { sp.SetIgnoreMeta(ignoreMetadata) sr, err := s.Get(sp) if err != nil { - return sr.HasMeta(), err + return err } obj = sr.Object() - return sr.HasMeta(), nil + return nil }) return obj, err } -func (e *StorageEngine) get(addr oid.Address, shardFunc func(s *shard.Shard, ignoreMetadata bool) (hasMetadata bool, err error)) error { +func (e *StorageEngine) get(addr oid.Address, shardFunc func(s *shard.Shard, ignoreMetadata bool) error) error { var ( hasDegraded bool shardWithMeta shardWrapper @@ -64,11 +64,11 @@ func (e *StorageEngine) get(addr oid.Address, shardFunc func(s *shard.Shard, ign noMeta := sh.GetMode().NoMetabase() hasDegraded = hasDegraded || noMeta - hasMetadata, err := shardFunc(sh.Shard, noMeta) + err := shardFunc(sh.Shard, noMeta) if err != nil { var siErr *objectSDK.SplitInfoError - if hasMetadata { + if errors.Is(err, shard.ErrMetaWithNoObject) { shardWithMeta = sh metaError = err } @@ -121,7 +121,7 @@ func (e *StorageEngine) get(addr oid.Address, shardFunc func(s *shard.Shard, ign continue } - _, err := shardFunc(sh.Shard, true) + err := shardFunc(sh.Shard, true) if shard.IsErrOutOfRange(err) { return apistatus.ObjectOutOfRange{} } @@ -151,13 +151,13 @@ func (e *StorageEngine) GetBytes(addr oid.Address) ([]byte, error) { b []byte err error ) - err = e.get(addr, func(s *shard.Shard, ignoreMetadata bool) (hasMetadata bool, err error) { + err = e.get(addr, func(s *shard.Shard, ignoreMetadata bool) error { if ignoreMetadata { b, err = s.GetBytes(addr) } else { - b, hasMetadata, err = s.GetBytesWithMetadataLookup(addr) + b, err = s.GetBytesWithMetadataLookup(addr) } - return + return err }) return b, err } diff --git a/pkg/local_object_storage/engine/range.go b/pkg/local_object_storage/engine/range.go index 55e0a2f3ab..5a6be52509 100644 --- a/pkg/local_object_storage/engine/range.go +++ b/pkg/local_object_storage/engine/range.go @@ -48,13 +48,13 @@ func (e *StorageEngine) GetRange(addr oid.Address, offset uint64, length uint64) shPrm.SetAddress(addr) shPrm.SetRange(offset, length) - err = e.get(addr, func(sh *shard.Shard, ignoreMetadata bool) (bool, error) { + err = e.get(addr, func(sh *shard.Shard, ignoreMetadata bool) error { shPrm.SetIgnoreMeta(ignoreMetadata) res, err := sh.GetRange(shPrm) if err == nil { data = res.Object().Payload() } - return res.HasMeta(), err + return err }) return data, err } diff --git a/pkg/local_object_storage/shard/get.go b/pkg/local_object_storage/shard/get.go index 78fef02b5d..f04efdbdd2 100644 --- a/pkg/local_object_storage/shard/get.go +++ b/pkg/local_object_storage/shard/get.go @@ -1,6 +1,7 @@ package shard import ( + "errors" "fmt" "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor" @@ -13,6 +14,9 @@ import ( "go.uber.org/zap" ) +// ErrMetaWithNoObject is returned when shard has metadata, but no object. +var ErrMetaWithNoObject = errors.New("got meta, but no object") + // GetPrm groups the parameters of Get operation. type GetPrm struct { addr oid.Address @@ -21,8 +25,7 @@ type GetPrm struct { // GetRes groups the resulting values of Get operation. type GetRes struct { - obj *objectSDK.Object - hasMeta bool + obj *objectSDK.Object } // SetAddress is a Get option to set the address of the requested object. @@ -43,11 +46,6 @@ func (r GetRes) Object() *objectSDK.Object { return r.obj } -// HasMeta returns true if info about the object was found in the metabase. -func (r GetRes) HasMeta() bool { - return r.hasMeta -} - // Get reads an object from shard. // // Returns any error encountered that @@ -85,8 +83,10 @@ func (s *Shard) Get(prm GetPrm) (GetRes, error) { } skipMeta := prm.skipMeta || s.info.Mode.NoMetabase() - var err error - res.hasMeta, err = s.fetchObjectData(prm.addr, skipMeta, cb, wc) + gotMeta, err := s.fetchObjectData(prm.addr, skipMeta, cb, wc) + if err != nil && gotMeta { + err = fmt.Errorf("%w, %w", err, ErrMetaWithNoObject) + } return res, err } @@ -160,18 +160,17 @@ func (s *Shard) fetchObjectData(addr oid.Address, skipMeta bool, // canonical NeoFS binary format. Returns [apistatus.ObjectNotFound] if object // is missing. func (s *Shard) GetBytes(addr oid.Address) ([]byte, error) { - b, _, err := s.getBytesWithMetadataLookup(addr, true) - return b, err + return s.getBytesWithMetadataLookup(addr, true) } // GetBytesWithMetadataLookup works similar to [shard.GetBytes], but pre-checks // object presence in the underlying metabase: if object cannot be accessed from // the metabase, GetBytesWithMetadataLookup returns an error. -func (s *Shard) GetBytesWithMetadataLookup(addr oid.Address) ([]byte, bool, error) { +func (s *Shard) GetBytesWithMetadataLookup(addr oid.Address) ([]byte, error) { return s.getBytesWithMetadataLookup(addr, false) } -func (s *Shard) getBytesWithMetadataLookup(addr oid.Address, skipMeta bool) ([]byte, bool, error) { +func (s *Shard) getBytesWithMetadataLookup(addr oid.Address, skipMeta bool) ([]byte, error) { s.m.RLock() defer s.m.RUnlock() @@ -185,5 +184,8 @@ func (s *Shard) getBytesWithMetadataLookup(addr oid.Address, skipMeta bool) ([]b b, err = w.GetBytes(addr) return err }) - return b, hasMeta, err + if err != nil && hasMeta { + err = fmt.Errorf("%w, %w", err, ErrMetaWithNoObject) + } + return b, err } diff --git a/pkg/local_object_storage/shard/get_test.go b/pkg/local_object_storage/shard/get_test.go index 93c7b10b4c..c3c6f0eec4 100644 --- a/pkg/local_object_storage/shard/get_test.go +++ b/pkg/local_object_storage/shard/get_test.go @@ -48,7 +48,6 @@ func testShardGet(t *testing.T, hasWriteCache bool) { res, err := testGet(t, sh, getPrm, hasWriteCache) require.NoError(t, err) require.Equal(t, obj, res.Object()) - require.True(t, res.HasMeta()) testGetBytes(t, sh, addr, obj.Marshal()) }) @@ -70,7 +69,6 @@ func testShardGet(t *testing.T, hasWriteCache bool) { res, err := testGet(t, sh, getPrm, hasWriteCache) require.NoError(t, err) require.Equal(t, obj, res.Object()) - require.True(t, res.HasMeta()) testGetBytes(t, sh, addr, obj.Marshal()) }) @@ -136,10 +134,9 @@ func testGetBytes(t testing.TB, sh *shard.Shard, addr oid.Address, objBin []byte require.NoError(t, err) require.Equal(t, objBin, b) - b, hasMeta, err := sh.GetBytesWithMetadataLookup(addr) + b, err = sh.GetBytesWithMetadataLookup(addr) require.NoError(t, err) require.Equal(t, objBin, b) - require.True(t, hasMeta) } // binary equal is used when object contains empty lists in the structure and diff --git a/pkg/local_object_storage/shard/range.go b/pkg/local_object_storage/shard/range.go index f45ae9432d..564bcb8361 100644 --- a/pkg/local_object_storage/shard/range.go +++ b/pkg/local_object_storage/shard/range.go @@ -1,6 +1,8 @@ package shard import ( + "fmt" + "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor" "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor/common" "github.com/nspcc-dev/neofs-node/pkg/local_object_storage/util/logicerr" @@ -23,8 +25,7 @@ type RngPrm struct { // RngRes groups the resulting values of GetRange operation. type RngRes struct { - obj *object.Object - hasMeta bool + obj *object.Object } // SetAddress is a Rng option to set the address of the requested object. @@ -52,11 +53,6 @@ func (r RngRes) Object() *object.Object { return r.obj } -// HasMeta returns true if info about the object was found in the metabase. -func (r RngRes) HasMeta() bool { - return r.hasMeta -} - // GetRange reads part of an object from shard. // // Returns any error encountered that @@ -109,8 +105,10 @@ func (s *Shard) GetRange(prm RngPrm) (RngRes, error) { } skipMeta := prm.skipMeta || s.info.Mode.NoMetabase() - var err error - res.hasMeta, err = s.fetchObjectData(prm.addr, skipMeta, cb, wc) + gotMeta, err := s.fetchObjectData(prm.addr, skipMeta, cb, wc) + if err != nil && gotMeta { + err = fmt.Errorf("%w, %w", err, ErrMetaWithNoObject) + } return res, err }