From 29fe8c41f33bff2668ab0a020a5401a07859fe38 Mon Sep 17 00:00:00 2001 From: Dmitrii Stepanov Date: Wed, 15 Nov 2023 14:18:03 +0300 Subject: [PATCH] [#655] storage: Drop `ErrorHandler` The only one usage was for logging. Now logging performed by storage anyway. Signed-off-by: Dmitrii Stepanov --- cmd/frostfs-node/config.go | 1 + .../blobstor/blobovniczatree/iterate.go | 14 ++++++++--- .../blobstor/common/iterate.go | 1 - .../blobstor/fstree/fstree.go | 23 ++++++++++++++----- .../blobstor/fstree/option.go | 8 +++++++ pkg/local_object_storage/blobstor/iterate.go | 15 ++++++------ .../blobstor/memstore/memstore.go | 3 --- 7 files changed, 45 insertions(+), 20 deletions(-) diff --git a/cmd/frostfs-node/config.go b/cmd/frostfs-node/config.go index a43b59ab2..3be33a2a1 100644 --- a/cmd/frostfs-node/config.go +++ b/cmd/frostfs-node/config.go @@ -818,6 +818,7 @@ func (c *cfg) getSubstorageOpts(shCfg shardCfg) []blobstor.SubStorage { fstree.WithPerm(sRead.perm), fstree.WithDepth(sRead.depth), fstree.WithNoSync(sRead.noSync), + fstree.WithLogger(c.log), } if c.metricsCollector != nil { fstreeOpts = append(fstreeOpts, diff --git a/pkg/local_object_storage/blobstor/blobovniczatree/iterate.go b/pkg/local_object_storage/blobstor/blobovniczatree/iterate.go index 7129e088a..d2b285c65 100644 --- a/pkg/local_object_storage/blobstor/blobovniczatree/iterate.go +++ b/pkg/local_object_storage/blobstor/blobovniczatree/iterate.go @@ -6,6 +6,7 @@ import ( "path/filepath" "time" + "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobovnicza" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" @@ -13,6 +14,7 @@ import ( "git.frostfs.info/TrueCloudLab/hrw" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // Iterate iterates over all objects in b. @@ -38,9 +40,11 @@ func (b *Blobovniczas) Iterate(ctx context.Context, prm common.IteratePrm) (comm data, err := b.compression.Decompress(elem.ObjectData()) if err != nil { if prm.IgnoreErrors { - if prm.ErrorHandler != nil { - return prm.ErrorHandler(elem.Address(), err) - } + b.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, + zap.Stringer("address", elem.Address()), + zap.String("err", err.Error()), + zap.String("storage_id", p), + zap.String("root_path", b.rootPath)) return nil } return fmt.Errorf("could not decompress object data: %w", err) @@ -70,6 +74,10 @@ func (b *Blobovniczas) iterateBlobovniczas(ctx context.Context, ignoreErrors boo blz, err := shBlz.Open() if err != nil { if ignoreErrors { + b.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, + zap.String("err", err.Error()), + zap.String("storage_id", p), + zap.String("root_path", b.rootPath)) return false, nil } return false, fmt.Errorf("could not open blobovnicza %s: %w", p, err) diff --git a/pkg/local_object_storage/blobstor/common/iterate.go b/pkg/local_object_storage/blobstor/common/iterate.go index 1fbb08d32..a1b8ff047 100644 --- a/pkg/local_object_storage/blobstor/common/iterate.go +++ b/pkg/local_object_storage/blobstor/common/iterate.go @@ -16,7 +16,6 @@ type IterationHandler func(IterationElement) error type IteratePrm struct { Handler IterationHandler IgnoreErrors bool - ErrorHandler func(oid.Address, error) error } // IterateRes groups the resulting values of Iterate operation. diff --git a/pkg/local_object_storage/blobstor/fstree/fstree.go b/pkg/local_object_storage/blobstor/fstree/fstree.go index 06309f53c..d1fe85901 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree.go @@ -14,10 +14,12 @@ import ( "syscall" "time" + "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/compression" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/util/logicerr" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" @@ -25,6 +27,7 @@ import ( oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) type keyLock interface { @@ -41,6 +44,8 @@ func (l *noopKeyLock) Unlock(string) {} type FSTree struct { Info + log *logger.Logger + *compression.Config Depth uint64 DirNameLen int @@ -86,6 +91,7 @@ func New(opts ...Option) *FSTree { metrics: &noopMetrics{}, fileGuard: &noopKeyLock{}, fileCounter: &noopCounter{}, + log: &logger.Logger{Logger: zap.L()}, } for i := range opts { opts[i](f) @@ -145,9 +151,13 @@ func (t *FSTree) Iterate(ctx context.Context, prm common.IteratePrm) (common.Ite func (t *FSTree) iterate(ctx context.Context, depth uint64, curPath []string, prm common.IteratePrm) error { curName := strings.Join(curPath[1:], "") - des, err := os.ReadDir(filepath.Join(curPath...)) + dirPath := filepath.Join(curPath...) + des, err := os.ReadDir(dirPath) if err != nil { if prm.IgnoreErrors { + t.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, + zap.String("err", err.Error()), + zap.String("directory_path", dirPath)) return nil } return err @@ -182,8 +192,8 @@ func (t *FSTree) iterate(ctx context.Context, depth uint64, curPath []string, pr if err != nil { continue } - - data, err := os.ReadFile(filepath.Join(curPath...)) + path := filepath.Join(curPath...) + data, err := os.ReadFile(path) if err != nil && os.IsNotExist(err) { continue } @@ -193,9 +203,10 @@ func (t *FSTree) iterate(ctx context.Context, depth uint64, curPath []string, pr } if err != nil { if prm.IgnoreErrors { - if prm.ErrorHandler != nil { - return prm.ErrorHandler(addr, err) - } + t.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, + zap.Stringer("address", addr), + zap.String("err", err.Error()), + zap.String("path", path)) continue } return err diff --git a/pkg/local_object_storage/blobstor/fstree/option.go b/pkg/local_object_storage/blobstor/fstree/option.go index 21d46ac4d..e6c7a2608 100644 --- a/pkg/local_object_storage/blobstor/fstree/option.go +++ b/pkg/local_object_storage/blobstor/fstree/option.go @@ -3,7 +3,9 @@ package fstree import ( "io/fs" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" utilSync "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/sync" + "go.uber.org/zap" ) type Option func(*FSTree) @@ -51,3 +53,9 @@ func WithFileCounter(c FileCounter) Option { f.fileGuard = utilSync.NewKeyLocker[string]() } } + +func WithLogger(l *logger.Logger) Option { + return func(f *FSTree) { + f.log = &logger.Logger{Logger: l.With(zap.String("component", "FSTree"))} + } +} diff --git a/pkg/local_object_storage/blobstor/iterate.go b/pkg/local_object_storage/blobstor/iterate.go index 5a41e4c4f..f213d7547 100644 --- a/pkg/local_object_storage/blobstor/iterate.go +++ b/pkg/local_object_storage/blobstor/iterate.go @@ -40,7 +40,14 @@ func (b *BlobStor) Iterate(ctx context.Context, prm common.IteratePrm) (common.I for i := range b.storage { _, err := b.storage[i].Storage.Iterate(ctx, prm) - if err != nil && !prm.IgnoreErrors { + if err != nil { + if prm.IgnoreErrors { + b.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, + zap.String("storage_path", b.storage[i].Storage.Path()), + zap.String("storage_type", b.storage[i].Storage.Type()), + zap.String("err", err.Error())) + continue + } return common.IterateRes{}, fmt.Errorf("blobstor iterator failure: %w", err) } } @@ -57,12 +64,6 @@ func IterateBinaryObjects(ctx context.Context, blz *BlobStor, f func(addr oid.Ad return f(elem.Address, elem.ObjectData, elem.StorageID) } prm.IgnoreErrors = true - prm.ErrorHandler = func(addr oid.Address, err error) error { - blz.log.Warn(logs.BlobstorErrorOccurredDuringTheIteration, - zap.Stringer("address", addr), - zap.String("err", err.Error())) - return nil - } _, err := blz.Iterate(ctx, prm) diff --git a/pkg/local_object_storage/blobstor/memstore/memstore.go b/pkg/local_object_storage/blobstor/memstore/memstore.go index 63e1a3e99..cc4f6921a 100644 --- a/pkg/local_object_storage/blobstor/memstore/memstore.go +++ b/pkg/local_object_storage/blobstor/memstore/memstore.go @@ -142,9 +142,6 @@ func (s *memstoreImpl) Iterate(_ context.Context, req common.IteratePrm) (common var err error if elem.ObjectData, err = s.compression.Decompress(elem.ObjectData); err != nil { if req.IgnoreErrors { - if req.ErrorHandler != nil { - return common.IterateRes{}, req.ErrorHandler(elem.Address, err) - } continue } return common.IterateRes{}, logicerr.Wrap(fmt.Errorf("(%T) decompressing data for address %q: %v", s, elem.Address.String(), err))