From d5194ab2a6dacd533daf9bc601622c4ef19b77fa Mon Sep 17 00:00:00 2001 From: Ekaterina Lebedeva Date: Tue, 19 Mar 2024 12:40:00 +0300 Subject: [PATCH] [#949] metabase: fix shard.UpdateID() metabase.Open() now reports metabase mode metric. shard.UpdateID() needs to read shard ID from metabase => needs to open metabase. It caused reporting 'shard undefined' metrics. To avoid reporting wrong metrics metabase.GetShardID() was added which also opens metabase and does not report metrics. Signed-off-by: Ekaterina Lebedeva --- pkg/local_object_storage/engine/shards.go | 4 +- pkg/local_object_storage/metabase/control.go | 9 ++- pkg/local_object_storage/metabase/shard_id.go | 63 +++++++++++++++---- .../metabase/version_test.go | 4 +- pkg/local_object_storage/shard/id.go | 27 +++----- 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/pkg/local_object_storage/engine/shards.go b/pkg/local_object_storage/engine/shards.go index 4bbf7eff..ccf2e9da 100644 --- a/pkg/local_object_storage/engine/shards.go +++ b/pkg/local_object_storage/engine/shards.go @@ -107,7 +107,7 @@ func (e *StorageEngine) AddShard(ctx context.Context, opts ...shard.Option) (*sh return sh.ID(), nil } -func (e *StorageEngine) createShard(ctx context.Context, opts []shard.Option) (*shard.Shard, error) { +func (e *StorageEngine) createShard(_ context.Context, opts []shard.Option) (*shard.Shard, error) { id, err := generateShardID() if err != nil { return nil, fmt.Errorf("could not generate shard ID: %w", err) @@ -126,7 +126,7 @@ func (e *StorageEngine) createShard(ctx context.Context, opts []shard.Option) (* shard.WithZeroCountCallback(e.processZeroCountContainers), )...) - if err := sh.UpdateID(ctx); err != nil { + if err := sh.UpdateID(); err != nil { e.log.Warn(logs.FailedToUpdateShardID, zap.Stringer("shard_id", sh.ID()), zap.String("metabase_path", sh.DumpInfo().MetaBaseInfo.Path), zap.Error(err)) } diff --git a/pkg/local_object_storage/metabase/control.go b/pkg/local_object_storage/metabase/control.go index f6eb35ee..cd53f0cd 100644 --- a/pkg/local_object_storage/metabase/control.go +++ b/pkg/local_object_storage/metabase/control.go @@ -202,11 +202,12 @@ func (db *DB) SyncCounters() error { })) } -// Close closes boltDB instance. +// Close closes boltDB instance +// and reports metabase metric. func (db *DB) Close() error { var err error if db.boltDB != nil { - err = metaerr.Wrap(db.boltDB.Close()) + err = db.close() } if err == nil { db.metrics.Close() @@ -214,6 +215,10 @@ func (db *DB) Close() error { return err } +func (db *DB) close() error { + return metaerr.Wrap(db.boltDB.Close()) +} + // Reload reloads part of the configuration. // It returns true iff database was reopened. // If a config option is invalid, it logs an error and returns nil. diff --git a/pkg/local_object_storage/metabase/shard_id.go b/pkg/local_object_storage/metabase/shard_id.go index c1ec8c67..7ae336a6 100644 --- a/pkg/local_object_storage/metabase/shard_id.go +++ b/pkg/local_object_storage/metabase/shard_id.go @@ -2,8 +2,11 @@ package meta import ( "bytes" + "errors" + "fmt" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/metaerr" + metamode "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard/mode" "go.etcd.io/bbolt" ) @@ -12,16 +15,32 @@ var ( shardIDKey = []byte("id") ) -// ReadShardID reads shard id from db. +// GetShardID sets metabase operation mode +// and reads shard id from db. // If id is missing, returns nil, nil. -func (db *DB) ReadShardID() ([]byte, error) { - db.modeMtx.RLock() - defer db.modeMtx.RUnlock() +// +// GetShardID does not report any metrics. +func (db *DB) GetShardID(mode metamode.Mode) ([]byte, error) { + db.modeMtx.Lock() + defer db.modeMtx.Unlock() + db.mode = mode - if db.mode.NoMetabase() { - return nil, ErrDegradedMode + if err := db.openDB(mode); err != nil { + return nil, fmt.Errorf("failed to open metabase: %w", err) } + id, err := db.readShardID() + + if cErr := db.close(); cErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close metabase: %w", cErr)) + } + + return id, metaerr.Wrap(err) +} + +// ReadShardID reads shard id from db. +// If id is missing, returns nil, nil. +func (db *DB) readShardID() ([]byte, error) { var id []byte err := db.boltDB.View(func(tx *bbolt.Tx) error { b := tx.Bucket(shardInfoBucket) @@ -33,17 +52,35 @@ func (db *DB) ReadShardID() ([]byte, error) { return id, metaerr.Wrap(err) } -// WriteShardID writes shard it to db. -func (db *DB) WriteShardID(id []byte) error { - db.modeMtx.RLock() - defer db.modeMtx.RUnlock() +// SetShardID sets metabase operation mode +// and writes shard id to db. +func (db *DB) SetShardID(id []byte, mode metamode.Mode) error { + db.modeMtx.Lock() + defer db.modeMtx.Unlock() + db.mode = mode - if db.mode.NoMetabase() { - return ErrDegradedMode - } else if db.mode.ReadOnly() { + if mode.ReadOnly() { return ErrReadOnlyMode } + if err := db.openDB(mode); err != nil { + return fmt.Errorf("failed to open metabase: %w", err) + } + + err := db.writeShardID(id) + if err == nil { + db.metrics.SetMode(mode) + } + + if cErr := db.close(); cErr != nil { + err = errors.Join(err, fmt.Errorf("failed to close metabase: %w", cErr)) + } + + return metaerr.Wrap(err) +} + +// writeShardID writes shard id to db. +func (db *DB) writeShardID(id []byte) error { return metaerr.Wrap(db.boltDB.Update(func(tx *bbolt.Tx) error { b, err := tx.CreateBucketIfNotExists(shardInfoBucket) if err != nil { diff --git a/pkg/local_object_storage/metabase/version_test.go b/pkg/local_object_storage/metabase/version_test.go index cea42da0..b2af428f 100644 --- a/pkg/local_object_storage/metabase/version_test.go +++ b/pkg/local_object_storage/metabase/version_test.go @@ -58,9 +58,7 @@ func TestVersion(t *testing.T) { }) t.Run("old data", func(t *testing.T) { db := newDB(t) - require.NoError(t, db.Open(context.Background(), mode.ReadWrite)) - require.NoError(t, db.WriteShardID([]byte{1, 2, 3, 4})) - require.NoError(t, db.Close()) + require.NoError(t, db.SetShardID([]byte{1, 2, 3, 4}, mode.ReadWrite)) require.NoError(t, db.Open(context.Background(), mode.ReadWrite)) require.NoError(t, db.Init()) diff --git a/pkg/local_object_storage/shard/id.go b/pkg/local_object_storage/shard/id.go index be474e0f..2fe68d27 100644 --- a/pkg/local_object_storage/shard/id.go +++ b/pkg/local_object_storage/shard/id.go @@ -1,9 +1,10 @@ package shard import ( - "context" + "errors" "fmt" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard/mode" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" "github.com/mr-tron/base58" "go.uber.org/zap" @@ -30,21 +31,11 @@ func (s *Shard) ID() *ID { } // UpdateID reads shard ID saved in the metabase and updates it if it is missing. -func (s *Shard) UpdateID(ctx context.Context) (err error) { +func (s *Shard) UpdateID() (err error) { var idFromMetabase []byte - metabaseOpened := !s.GetMode().NoMetabase() - if err = s.metaBase.Open(ctx, s.GetMode()); err != nil { - err = fmt.Errorf("failed to open metabase: %w", err) - metabaseOpened = false - } - if metabaseOpened { - defer func() { - cErr := s.metaBase.Close() - if cErr != nil { - err = fmt.Errorf("failed to close metabase: %w", cErr) - } - }() - if idFromMetabase, err = s.metaBase.ReadShardID(); err != nil { + modeDegraded := s.GetMode().NoMetabase() + if !modeDegraded { + if idFromMetabase, err = s.metaBase.GetShardID(mode.ReadOnly); err != nil { err = fmt.Errorf("failed to read shard id from metabase: %w", err) } } @@ -73,9 +64,9 @@ func (s *Shard) UpdateID(ctx context.Context) (err error) { s.pilorama.SetParentID(s.info.ID.String()) } - if len(idFromMetabase) == 0 && metabaseOpened { - if err = s.metaBase.WriteShardID(*s.info.ID); err != nil { - err = fmt.Errorf("failed to write shard id to metabase: %w", err) + if len(idFromMetabase) == 0 && !modeDegraded { + if setErr := s.metaBase.SetShardID(*s.info.ID, s.GetMode()); setErr != nil { + err = errors.Join(err, fmt.Errorf("failed to write shard id to metabase: %w", setErr)) } } return