From e2a6663ef6246b6f8b06dc6b18f2f1cef1f7e273 Mon Sep 17 00:00:00 2001 From: Aleksey Savchuk Date: Tue, 17 Dec 2024 13:40:37 +0300 Subject: [PATCH] [#1445] shard/gc: Make handler and callback names more descriptive Consider the following methods: - `(*Shard).collectExpiredTombstones` - `(*StorageEngines).processExpiredTombstones` - `(*Shard).HandleExpiredTombstones` All of them handle not tombstones but graves. `HandleExpiredTombstones` in fact deletes tombstones but it does it based on graves it received. So, rename all `...Tombstones` methods to `...Graves` method. It'll make future changes in the garbage collector behavior simpler. Also, rename all `...Locks` methods to `...LockObjects` because they handle not locks but lock objects. Signed-off-by: Aleksey Savchuk --- pkg/local_object_storage/engine/inhume.go | 8 ++++---- pkg/local_object_storage/engine/shards.go | 4 ++-- pkg/local_object_storage/shard/control.go | 4 ++-- pkg/local_object_storage/shard/gc.go | 18 +++++++++--------- .../shard/gc_internal_test.go | 2 +- pkg/local_object_storage/shard/shard.go | 17 ++++++++++------- pkg/local_object_storage/shard/shard_test.go | 2 +- 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/pkg/local_object_storage/engine/inhume.go b/pkg/local_object_storage/engine/inhume.go index 4cba35482..636a082c7 100644 --- a/pkg/local_object_storage/engine/inhume.go +++ b/pkg/local_object_storage/engine/inhume.go @@ -285,9 +285,9 @@ func (e *StorageEngine) GetLocks(ctx context.Context, addr oid.Address) ([]Lock, return allLocks, outErr } -func (e *StorageEngine) processExpiredTombstones(ctx context.Context, addrs []meta.TombstonedObject) { +func (e *StorageEngine) processExpiredGraves(ctx context.Context, addrs []meta.TombstonedObject) { e.iterateOverUnsortedShards(func(sh hashedShard) (stop bool) { - sh.HandleExpiredTombstones(ctx, addrs) + sh.HandleExpiredGraves(ctx, addrs) select { case <-ctx.Done(): @@ -298,9 +298,9 @@ func (e *StorageEngine) processExpiredTombstones(ctx context.Context, addrs []me }) } -func (e *StorageEngine) processExpiredLocks(ctx context.Context, epoch uint64, lockers []oid.Address) { +func (e *StorageEngine) processExpiredLockObjects(ctx context.Context, epoch uint64, lockers []oid.Address) { e.iterateOverUnsortedShards(func(sh hashedShard) (stop bool) { - sh.HandleExpiredLocks(ctx, epoch, lockers) + sh.HandleExpiredLockObjects(ctx, epoch, lockers) select { case <-ctx.Done(): diff --git a/pkg/local_object_storage/engine/shards.go b/pkg/local_object_storage/engine/shards.go index 898f685ec..f15054876 100644 --- a/pkg/local_object_storage/engine/shards.go +++ b/pkg/local_object_storage/engine/shards.go @@ -131,8 +131,8 @@ func (e *StorageEngine) createShard(ctx context.Context, opts []shard.Option) (* sh := shard.New(append(opts, shard.WithID(id), - shard.WithExpiredTombstonesCallback(e.processExpiredTombstones), - shard.WithExpiredLocksCallback(e.processExpiredLocks), + shard.WithExpiredTombstonesCallback(e.processExpiredGraves), + shard.WithExpiredLocksCallback(e.processExpiredLockObjects), shard.WithDeletedLockCallback(e.processDeletedLocks), shard.WithReportErrorFunc(e.reportShardErrorByID), shard.WithZeroSizeCallback(e.processZeroSizeContainers), diff --git a/pkg/local_object_storage/shard/control.go b/pkg/local_object_storage/shard/control.go index 7734c4d92..cfa76ad5f 100644 --- a/pkg/local_object_storage/shard/control.go +++ b/pkg/local_object_storage/shard/control.go @@ -116,9 +116,9 @@ func (s *Shard) Init(ctx context.Context) error { eventNewEpoch: { cancelFunc: func() {}, handlers: []eventHandler{ - s.collectExpiredLocks, + s.collectExpiredLockObjects, s.collectExpiredObjects, - s.collectExpiredTombstones, + s.collectExpiredGraves, s.collectExpiredMetrics, }, }, diff --git a/pkg/local_object_storage/shard/gc.go b/pkg/local_object_storage/shard/gc.go index 1b218a372..f0ca4446a 100644 --- a/pkg/local_object_storage/shard/gc.go +++ b/pkg/local_object_storage/shard/gc.go @@ -462,7 +462,7 @@ func (s *Shard) getExpiredWithLinked(ctx context.Context, source []oid.Address) return result, nil } -func (s *Shard) collectExpiredTombstones(ctx context.Context, e Event) { +func (s *Shard) collectExpiredGraves(ctx context.Context, e Event) { var err error startedAt := time.Now() @@ -526,7 +526,7 @@ func (s *Shard) collectExpiredTombstones(ctx context.Context, e Event) { log.Debug(ctx, logs.ShardHandlingExpiredTombstonesBatch, zap.Int("number", len(tssExp))) if len(tssExp) > 0 { - s.expiredTombstonesCallback(ctx, tssExp) + s.expiredGravesCallback(ctx, tssExp) } iterPrm.SetOffset(tss[tssLen-1].Address()) @@ -535,7 +535,7 @@ func (s *Shard) collectExpiredTombstones(ctx context.Context, e Event) { } } -func (s *Shard) collectExpiredLocks(ctx context.Context, e Event) { +func (s *Shard) collectExpiredLockObjects(ctx context.Context, e Event) { var err error startedAt := time.Now() @@ -561,7 +561,7 @@ func (s *Shard) collectExpiredLocks(ctx context.Context, e Event) { if len(batch) == batchSize { expired := batch errGroup.Go(func() error { - s.expiredLocksCallback(egCtx, e.(newEpoch).epoch, expired) + s.expiredLockObjectsCallback(egCtx, e.(newEpoch).epoch, expired) return egCtx.Err() }) batch = make([]oid.Address, 0, batchSize) @@ -575,7 +575,7 @@ func (s *Shard) collectExpiredLocks(ctx context.Context, e Event) { if len(batch) > 0 { expired := batch errGroup.Go(func() error { - s.expiredLocksCallback(egCtx, e.(newEpoch).epoch, expired) + s.expiredLockObjectsCallback(egCtx, e.(newEpoch).epoch, expired) return egCtx.Err() }) } @@ -622,11 +622,11 @@ func (s *Shard) selectExpired(ctx context.Context, epoch uint64, addresses []oid return s.metaBase.FilterExpired(ctx, epoch, addresses) } -// HandleExpiredTombstones marks tombstones themselves as garbage +// HandleExpiredGraves marks tombstones themselves as garbage // and clears up corresponding graveyard records. // // Does not modify tss. -func (s *Shard) HandleExpiredTombstones(ctx context.Context, tss []meta.TombstonedObject) { +func (s *Shard) HandleExpiredGraves(ctx context.Context, tss []meta.TombstonedObject) { s.m.RLock() defer s.m.RUnlock() @@ -656,9 +656,9 @@ func (s *Shard) HandleExpiredTombstones(ctx context.Context, tss []meta.Tombston } } -// HandleExpiredLocks unlocks all objects which were locked by lockers. +// HandleExpiredLockObjects unlocks all objects which were locked by lockers. // If successful, marks lockers themselves as garbage. -func (s *Shard) HandleExpiredLocks(ctx context.Context, epoch uint64, lockers []oid.Address) { +func (s *Shard) HandleExpiredLockObjects(ctx context.Context, epoch uint64, lockers []oid.Address) { if s.GetMode().NoMetabase() { return } diff --git a/pkg/local_object_storage/shard/gc_internal_test.go b/pkg/local_object_storage/shard/gc_internal_test.go index 9998bbae2..681ad9ffa 100644 --- a/pkg/local_object_storage/shard/gc_internal_test.go +++ b/pkg/local_object_storage/shard/gc_internal_test.go @@ -65,7 +65,7 @@ func Test_ObjectNotFoundIfNotDeletedFromMetabase(t *testing.T) { sh.HandleDeletedLocks(ctx, addresses) }), WithExpiredLocksCallback(func(ctx context.Context, epoch uint64, a []oid.Address) { - sh.HandleExpiredLocks(ctx, epoch, a) + sh.HandleExpiredLockObjects(ctx, epoch, a) }), WithGCWorkerPoolInitializer(func(sz int) util.WorkerPool { pool, err := ants.NewPool(sz) diff --git a/pkg/local_object_storage/shard/shard.go b/pkg/local_object_storage/shard/shard.go index 1eb7f14d0..9c189382c 100644 --- a/pkg/local_object_storage/shard/shard.go +++ b/pkg/local_object_storage/shard/shard.go @@ -46,8 +46,11 @@ type Shard struct { // Option represents Shard's constructor option. type Option func(*cfg) -// ExpiredTombstonesCallback is a callback handling list of expired tombstones. -type ExpiredTombstonesCallback func(context.Context, []meta.TombstonedObject) +// ExpiredGravesCallback is a callback handling list of expired graves. +// +// Need to mention, that [ExpiredGravesCallback] handles exactly graves, +// not tombstones. [ExpiredObjectsCallback] can be used to handle tombstones. +type ExpiredGravesCallback func(context.Context, []meta.TombstonedObject) // ExpiredObjectsCallback is a callback handling list of expired objects. type ExpiredObjectsCallback func(context.Context, uint64, []oid.Address) @@ -82,9 +85,9 @@ type cfg struct { gcCfg gcCfg - expiredTombstonesCallback ExpiredTombstonesCallback + expiredGravesCallback ExpiredGravesCallback - expiredLocksCallback ExpiredObjectsCallback + expiredLockObjectsCallback ExpiredObjectsCallback deletedLockCallBack DeletedLockCallback @@ -248,9 +251,9 @@ func WithGCRemoverSleepInterval(dur time.Duration) Option { // WithExpiredTombstonesCallback returns option to specify callback // of the expired tombstones handler. -func WithExpiredTombstonesCallback(cb ExpiredTombstonesCallback) Option { +func WithExpiredTombstonesCallback(cb ExpiredGravesCallback) Option { return func(c *cfg) { - c.expiredTombstonesCallback = cb + c.expiredGravesCallback = cb } } @@ -258,7 +261,7 @@ func WithExpiredTombstonesCallback(cb ExpiredTombstonesCallback) Option { // of the expired LOCK objects handler. func WithExpiredLocksCallback(cb ExpiredObjectsCallback) Option { return func(c *cfg) { - c.expiredLocksCallback = cb + c.expiredLockObjectsCallback = cb } } diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index f9ee34488..7515eef39 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -93,7 +93,7 @@ func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard sh.HandleDeletedLocks(ctx, addresses) }), WithExpiredLocksCallback(func(ctx context.Context, epoch uint64, a []oid.Address) { - sh.HandleExpiredLocks(ctx, epoch, a) + sh.HandleExpiredLockObjects(ctx, epoch, a) }), WithGCWorkerPoolInitializer(func(sz int) util.WorkerPool { pool, err := ants.NewPool(sz)