From b74189f7dc1f2e2a36f5b9ea640aefd7c02ccd5c Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 13 Nov 2024 15:56:16 +0300 Subject: [PATCH] [#1493] metabase: Merge Inhume() and DropGraves() for tombstones DropGraves() is only used to drop gravemarks after a tombstone removal. Thus, it makes sense to do Inhume() and DropGraves() in one transaction. It has less overhead and no unexpected problems in case of sudden power failure. Signed-off-by: Evgenii Stratonikov --- .../metabase/delete_ec_test.go | 20 +--------- .../metabase/graveyard.go | 39 ++++++++++++------- .../metabase/graveyard_test.go | 26 ++++++++++--- pkg/local_object_storage/shard/gc.go | 21 +--------- 4 files changed, 50 insertions(+), 56 deletions(-) diff --git a/pkg/local_object_storage/metabase/delete_ec_test.go b/pkg/local_object_storage/metabase/delete_ec_test.go index 9f1f91e14..884da23ff 100644 --- a/pkg/local_object_storage/metabase/delete_ec_test.go +++ b/pkg/local_object_storage/metabase/delete_ec_test.go @@ -130,17 +130,9 @@ func TestDeleteECObject_WithoutSplit(t *testing.T) { require.NoError(t, db.IterateOverGraveyard(context.Background(), graveyardIterationPrm)) require.Equal(t, 2, len(tombstonedObjects)) - var tombstones []oid.Address - for _, tss := range tombstonedObjects { - tombstones = append(tombstones, tss.tomb) - } - inhumePrm.SetAddresses(tombstones...) - inhumePrm.SetGCMark() - _, err = db.Inhume(context.Background(), inhumePrm) + _, err = db.InhumeTombstones(context.Background(), tombstonedObjects) require.NoError(t, err) - require.NoError(t, db.DropGraves(context.Background(), tombstonedObjects)) - // GC finds tombstone as garbage and deletes it garbageAddresses = nil @@ -374,17 +366,9 @@ func testDeleteECObjectWithSplit(t *testing.T, chunksCount int, withLinking bool require.NoError(t, db.IterateOverGraveyard(context.Background(), graveyardIterationPrm)) require.True(t, len(tombstonedObjects) == parentCount+chunksCount) - var tombstones []oid.Address - for _, tss := range tombstonedObjects { - tombstones = append(tombstones, tss.tomb) - } - inhumePrm.SetAddresses(tombstones...) - inhumePrm.SetGCMark() - _, err = db.Inhume(context.Background(), inhumePrm) + _, err = db.InhumeTombstones(context.Background(), tombstonedObjects) require.NoError(t, err) - require.NoError(t, db.DropGraves(context.Background(), tombstonedObjects)) - // GC finds tombstone as garbage and deletes it garbageAddresses = nil diff --git a/pkg/local_object_storage/metabase/graveyard.go b/pkg/local_object_storage/metabase/graveyard.go index 31f95d6ed..b0db952b2 100644 --- a/pkg/local_object_storage/metabase/graveyard.go +++ b/pkg/local_object_storage/metabase/graveyard.go @@ -9,6 +9,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/metaerr" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" + cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "go.etcd.io/bbolt" ) @@ -255,46 +256,58 @@ func graveFromKV(k, v []byte) (res TombstonedObject, err error) { return } -// DropGraves deletes tombstoned objects from the +// InhumeTombstones deletes tombstoned objects from the // graveyard bucket. // // Returns any error appeared during deletion process. -func (db *DB) DropGraves(ctx context.Context, tss []TombstonedObject) error { +func (db *DB) InhumeTombstones(ctx context.Context, tss []TombstonedObject) (InhumeRes, error) { var ( startedAt = time.Now() success = false ) defer func() { - db.metrics.AddMethodDuration("DropGraves", time.Since(startedAt), success) + db.metrics.AddMethodDuration("InhumeTombstones", time.Since(startedAt), success) }() - _, span := tracing.StartSpanFromContext(ctx, "metabase.DropGraves") + _, span := tracing.StartSpanFromContext(ctx, "metabase.InhumeTombstones") defer span.End() db.modeMtx.RLock() defer db.modeMtx.RUnlock() if db.mode.NoMetabase() { - return ErrDegradedMode + return InhumeRes{}, ErrDegradedMode } else if db.mode.ReadOnly() { - return ErrReadOnlyMode + return InhumeRes{}, ErrReadOnlyMode } buf := make([]byte, addressKeySize) + prm := InhumePrm{forceRemoval: true} + currEpoch := db.epochState.CurrentEpoch() - return db.boltDB.Batch(func(tx *bbolt.Tx) error { - bkt := tx.Bucket(graveyardBucketName) - if bkt == nil { - return nil + var res InhumeRes + + err := db.boltDB.Batch(func(tx *bbolt.Tx) error { + res = InhumeRes{inhumedByCnrID: make(map[cid.ID]ObjectCounters)} + + garbageBKT := tx.Bucket(garbageBucketName) + graveyardBKT := tx.Bucket(graveyardBucketName) + + bkt, value, err := db.getInhumeTargetBucketAndValue(garbageBKT, graveyardBKT, prm) + if err != nil { + return err } - for _, ts := range tss { - err := bkt.Delete(addressKey(ts.Address(), buf)) - if err != nil { + for i := range tss { + if err := db.inhumeTxSingle(bkt, value, graveyardBKT, garbageBKT, tss[i].Tombstone(), buf, currEpoch, prm, &res); err != nil { + return err + } + if err := graveyardBKT.Delete(addressKey(tss[i].Address(), buf)); err != nil { return err } } return nil }) + return res, err } diff --git a/pkg/local_object_storage/metabase/graveyard_test.go b/pkg/local_object_storage/metabase/graveyard_test.go index 99794e609..ebadecc04 100644 --- a/pkg/local_object_storage/metabase/graveyard_test.go +++ b/pkg/local_object_storage/metabase/graveyard_test.go @@ -7,7 +7,9 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" meta "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase" + cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" + objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" oidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id/test" "github.com/stretchr/testify/require" @@ -393,7 +395,7 @@ func TestDB_IterateOverGarbage_Offset(t *testing.T) { require.False(t, iWasCalled) } -func TestDB_DropGraves(t *testing.T) { +func TestDB_InhumeTombstones(t *testing.T) { db := newDB(t) defer func() { require.NoError(t, db.Close(context.Background())) }() @@ -410,9 +412,20 @@ func TestDB_DropGraves(t *testing.T) { err = putBig(db, obj2) require.NoError(t, err) - // inhume with tombstone - addrTombstone := oidtest.Address() - addrTombstone.SetContainer(cnr) + id1, _ := obj1.ID() + id2, _ := obj2.ID() + ts := objectSDK.NewTombstone() + ts.SetMembers([]oid.ID{id1, id2}) + objTs := objectSDK.New() + objTs.SetContainerID(cnr) + objTs.SetType(objectSDK.TypeTombstone) + + data, _ := ts.Marshal() + objTs.SetPayload(data) + require.NoError(t, objectSDK.CalculateAndSetID(objTs)) + require.NoError(t, putBig(db, objTs)) + + addrTombstone := object.AddressOf(objTs) var inhumePrm meta.InhumePrm inhumePrm.SetAddresses(object.AddressOf(obj1), object.AddressOf(obj2)) @@ -435,8 +448,11 @@ func TestDB_DropGraves(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, counter) - err = db.DropGraves(context.Background(), buriedTS) + res, err := db.InhumeTombstones(context.Background(), buriedTS) require.NoError(t, err) + require.EqualValues(t, 1, res.LogicInhumed()) + require.EqualValues(t, 0, res.UserInhumed()) + require.EqualValues(t, map[cid.ID]meta.ObjectCounters{cnr: {Logic: 1}}, res.InhumedByCnrID()) counter = 0 iterGravePRM.SetHandler(func(_ meta.TombstonedObject) error { diff --git a/pkg/local_object_storage/shard/gc.go b/pkg/local_object_storage/shard/gc.go index 57c21459c..c212f8c36 100644 --- a/pkg/local_object_storage/shard/gc.go +++ b/pkg/local_object_storage/shard/gc.go @@ -634,19 +634,7 @@ func (s *Shard) HandleExpiredTombstones(ctx context.Context, tss []meta.Tombston return } - // Mark tombstones as garbage. - var pInhume meta.InhumePrm - - tsAddrs := make([]oid.Address, 0, len(tss)) - for _, ts := range tss { - tsAddrs = append(tsAddrs, ts.Tombstone()) - } - - pInhume.SetGCMark() - pInhume.SetAddresses(tsAddrs...) - - // inhume tombstones - res, err := s.metaBase.Inhume(ctx, pInhume) + res, err := s.metaBase.InhumeTombstones(ctx, tss) if err != nil { s.log.Warn(ctx, logs.ShardCouldNotMarkTombstonesAsGarbage, zap.String("error", err.Error()), @@ -666,13 +654,6 @@ func (s *Shard) HandleExpiredTombstones(ctx context.Context, tss []meta.Tombston s.addToContainerSize(delInfo.CID.EncodeToString(), -int64(delInfo.Size)) i++ } - - // drop just processed expired tombstones - // from graveyard - err = s.metaBase.DropGraves(ctx, tss) - if err != nil { - s.log.Warn(ctx, logs.ShardCouldNotDropExpiredGraveRecords, zap.Error(err)) - } } // HandleExpiredLocks unlocks all objects which were locked by lockers.