Skip to content

Commit

Permalink
shard: pass meta problem as error, drop HasMeta() from *Res
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
roman-khimov committed Nov 22, 2024
1 parent 7c628a0 commit cfce9e9
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 39 deletions.
20 changes: 10 additions & 10 deletions pkg/local_object_storage/engine/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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{}
}
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions pkg/local_object_storage/engine/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
30 changes: 16 additions & 14 deletions pkg/local_object_storage/shard/get.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package shard

import (
"errors"
"fmt"

"github.com/nspcc-dev/neofs-node/pkg/local_object_storage/blobstor"
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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()

Expand All @@ -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
}
5 changes: 1 addition & 4 deletions pkg/local_object_storage/shard/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand All @@ -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())
})
Expand Down Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions pkg/local_object_storage/shard/range.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

0 comments on commit cfce9e9

Please sign in to comment.