From e5e56e9b8a9e6bf24fb8358901c97718515a7f12 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 30 Aug 2023 17:10:21 +0300 Subject: [PATCH 01/11] [#668] shard/test: Fix typo in `existence` Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc_internal_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/local_object_storage/shard/gc_internal_test.go b/pkg/local_object_storage/shard/gc_internal_test.go index c258b6c99..9a4f52029 100644 --- a/pkg/local_object_storage/shard/gc_internal_test.go +++ b/pkg/local_object_storage/shard/gc_internal_test.go @@ -116,13 +116,13 @@ func Test_ObjectNotFoundIfNotDeletedFromMetabase(t *testing.T) { storageID, err := sh.metaBase.StorageID(context.Background(), metaStIDPrm) require.NoError(t, err, "failed to get storage ID") - //check existance in blobstore + //check existence in blobstore var bsExisted common.ExistsPrm bsExisted.Address = addr bsExisted.StorageID = storageID.StorageID() exRes, err := sh.blobStor.Exists(context.Background(), bsExisted) - require.NoError(t, err, "failed to check blobstore existance") - require.True(t, exRes.Exists, "invalid blobstore existance result") + require.NoError(t, err, "failed to check blobstore existence") + require.True(t, exRes.Exists, "invalid blobstore existence result") //drop from blobstor var bsDeletePrm common.DeletePrm @@ -131,10 +131,10 @@ func Test_ObjectNotFoundIfNotDeletedFromMetabase(t *testing.T) { _, err = sh.blobStor.Delete(context.Background(), bsDeletePrm) require.NoError(t, err, "failed to delete from blobstore") - //check existance in blobstore + //check existence in blobstore exRes, err = sh.blobStor.Exists(context.Background(), bsExisted) - require.NoError(t, err, "failed to check blobstore existance") - require.False(t, exRes.Exists, "invalid blobstore existance result") + require.NoError(t, err, "failed to check blobstore existence") + require.False(t, exRes.Exists, "invalid blobstore existence result") //get should return object not found _, err = sh.Get(context.Background(), getPrm) -- 2.45.3 From 97cac8720357ab1a00fcab61ac21b7c98ff7efa8 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 30 Aug 2023 18:25:56 +0300 Subject: [PATCH 02/11] [#668] shard/test: Remove subtest from TestCounters Otherwise, individual tests cannot be run. Signed-off-by: Evgenii Stratonikov --- .../shard/metrics_test.go | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/local_object_storage/shard/metrics_test.go b/pkg/local_object_storage/shard/metrics_test.go index 3d904331e..280741f7b 100644 --- a/pkg/local_object_storage/shard/metrics_test.go +++ b/pkg/local_object_storage/shard/metrics_test.go @@ -163,21 +163,19 @@ func TestCounters(t *testing.T) { totalPayload += oSize } - t.Run("put", func(t *testing.T) { - var prm shard.PutPrm + var prm shard.PutPrm - for i := 0; i < objNumber; i++ { - prm.SetObject(oo[i]) + for i := 0; i < objNumber; i++ { + prm.SetObject(oo[i]) - _, err := sh.Put(context.Background(), prm) - require.NoError(t, err) - } + _, err := sh.Put(context.Background(), prm) + require.NoError(t, err) + } - require.Equal(t, uint64(objNumber), mm.getObjectCounter(physical)) - require.Equal(t, uint64(objNumber), mm.getObjectCounter(logical)) - require.Equal(t, expectedSizes, mm.containerSizes()) - require.Equal(t, totalPayload, mm.payloadSize()) - }) + require.Equal(t, uint64(objNumber), mm.getObjectCounter(physical)) + require.Equal(t, uint64(objNumber), mm.getObjectCounter(logical)) + require.Equal(t, expectedSizes, mm.containerSizes()) + require.Equal(t, totalPayload, mm.payloadSize()) t.Run("inhume_GC", func(t *testing.T) { var prm shard.InhumePrm -- 2.45.3 From b532c99f9c6796d76ea12478647dd9ee5620f8bf Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 31 Aug 2023 15:47:06 +0300 Subject: [PATCH 03/11] [#668] shard/test: Move tests to the main package Semantic patch (also, duplicate definitions are removed): ``` @@ var e identifier @@ -import "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" -shard.e +e ``` Signed-off-by: Evgenii Stratonikov --- .../shard/control_test.go | 6 --- pkg/local_object_storage/shard/delete_test.go | 11 +++--- pkg/local_object_storage/shard/gc_test.go | 15 ++++---- pkg/local_object_storage/shard/get_test.go | 9 ++--- pkg/local_object_storage/shard/head_test.go | 9 ++--- pkg/local_object_storage/shard/inhume_test.go | 9 ++--- pkg/local_object_storage/shard/list_test.go | 7 ++-- pkg/local_object_storage/shard/lock_test.go | 33 ++++++++--------- .../shard/metrics_test.go | 28 ++++++-------- pkg/local_object_storage/shard/range_test.go | 7 ++-- pkg/local_object_storage/shard/shard_test.go | 37 +++++++++---------- .../shard/shutdown_test.go | 7 ++-- 12 files changed, 79 insertions(+), 99 deletions(-) diff --git a/pkg/local_object_storage/shard/control_test.go b/pkg/local_object_storage/shard/control_test.go index 73612d840..128e5cc0c 100644 --- a/pkg/local_object_storage/shard/control_test.go +++ b/pkg/local_object_storage/shard/control_test.go @@ -32,12 +32,6 @@ import ( "go.etcd.io/bbolt" ) -type epochState struct{} - -func (s epochState) CurrentEpoch() uint64 { - return 0 -} - type objAddr struct { obj *objectSDK.Object addr oid.Address diff --git a/pkg/local_object_storage/shard/delete_test.go b/pkg/local_object_storage/shard/delete_test.go index bfafcdc74..8ee613186 100644 --- a/pkg/local_object_storage/shard/delete_test.go +++ b/pkg/local_object_storage/shard/delete_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -7,7 +7,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" "github.com/stretchr/testify/require" @@ -36,8 +35,8 @@ func testShardDelete(t *testing.T, hasWriteCache bool) { obj := testutil.GenerateObjectWithCID(cnr) testutil.AddAttribute(obj, "foo", "bar") - var putPrm shard.PutPrm - var getPrm shard.GetPrm + var putPrm PutPrm + var getPrm GetPrm t.Run("big object", func(t *testing.T) { testutil.AddPayload(obj, 1<<20) @@ -45,7 +44,7 @@ func testShardDelete(t *testing.T, hasWriteCache bool) { putPrm.SetObject(obj) getPrm.SetAddress(object.AddressOf(obj)) - var delPrm shard.DeletePrm + var delPrm DeletePrm delPrm.SetAddresses(object.AddressOf(obj)) _, err := sh.Put(context.Background(), putPrm) @@ -71,7 +70,7 @@ func testShardDelete(t *testing.T, hasWriteCache bool) { putPrm.SetObject(obj) getPrm.SetAddress(object.AddressOf(obj)) - var delPrm shard.DeletePrm + var delPrm DeletePrm delPrm.SetAddresses(object.AddressOf(obj)) _, err := sh.Put(context.Background(), putPrm) diff --git a/pkg/local_object_storage/shard/gc_test.go b/pkg/local_object_storage/shard/gc_test.go index 9d2771ae4..24038f10b 100644 --- a/pkg/local_object_storage/shard/gc_test.go +++ b/pkg/local_object_storage/shard/gc_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -10,7 +10,6 @@ import ( objectCore "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" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" writecacheconfig "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/config" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" @@ -55,7 +54,7 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { lock.SetAttributes(lockExpirationAttr) lockID, _ := lock.ID() - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) @@ -69,9 +68,9 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { require.NoError(t, err) epoch.Value = 105 - sh.NotificationChannel() <- shard.EventNewEpoch(epoch.Value) + sh.NotificationChannel() <- EventNewEpoch(epoch.Value) - var getPrm shard.GetPrm + var getPrm GetPrm getPrm.SetAddress(objectCore.AddressOf(obj)) require.Eventually(t, func() bool { _, err = sh.Get(context.Background(), getPrm) @@ -141,7 +140,7 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { lock.SetAttributes(lockExpirationAttr) lockID, _ := lock.ID() - var putPrm shard.PutPrm + var putPrm PutPrm for _, child := range children { putPrm.SetObject(child) @@ -160,7 +159,7 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { _, err = sh.Put(context.Background(), putPrm) require.NoError(t, err) - var getPrm shard.GetPrm + var getPrm GetPrm getPrm.SetAddress(objectCore.AddressOf(parent)) _, err = sh.Get(context.Background(), getPrm) @@ -168,7 +167,7 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { require.True(t, errors.As(err, &splitInfoError), "split info must be provided") epoch.Value = 105 - sh.NotificationChannel() <- shard.EventNewEpoch(epoch.Value) + sh.NotificationChannel() <- EventNewEpoch(epoch.Value) require.Eventually(t, func() bool { _, err = sh.Get(context.Background(), getPrm) diff --git a/pkg/local_object_storage/shard/get_test.go b/pkg/local_object_storage/shard/get_test.go index 25639902d..d3aeb5339 100644 --- a/pkg/local_object_storage/shard/get_test.go +++ b/pkg/local_object_storage/shard/get_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "bytes" @@ -9,7 +9,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -35,8 +34,8 @@ func testShardGet(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) defer releaseShard(sh, t) - var putPrm shard.PutPrm - var getPrm shard.GetPrm + var putPrm PutPrm + var getPrm GetPrm t.Run("small object", func(t *testing.T) { obj := testutil.GenerateObject() @@ -116,7 +115,7 @@ func testShardGet(t *testing.T, hasWriteCache bool) { }) } -func testGet(t *testing.T, sh *shard.Shard, getPrm shard.GetPrm, hasWriteCache bool) (shard.GetRes, error) { +func testGet(t *testing.T, sh *Shard, getPrm GetPrm, hasWriteCache bool) (GetRes, error) { res, err := sh.Get(context.Background(), getPrm) if hasWriteCache { require.Eventually(t, func() bool { diff --git a/pkg/local_object_storage/shard/head_test.go b/pkg/local_object_storage/shard/head_test.go index 3a7457ace..bf6ba990b 100644 --- a/pkg/local_object_storage/shard/head_test.go +++ b/pkg/local_object_storage/shard/head_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -8,7 +8,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -33,8 +32,8 @@ func testShardHead(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) defer releaseShard(sh, t) - var putPrm shard.PutPrm - var headPrm shard.HeadPrm + var putPrm PutPrm + var headPrm HeadPrm t.Run("regular object", func(t *testing.T) { obj := testutil.GenerateObject() @@ -87,7 +86,7 @@ func testShardHead(t *testing.T, hasWriteCache bool) { }) } -func testHead(t *testing.T, sh *shard.Shard, headPrm shard.HeadPrm, hasWriteCache bool) (shard.HeadRes, error) { +func testHead(t *testing.T, sh *Shard, headPrm HeadPrm, hasWriteCache bool) (HeadRes, error) { res, err := sh.Head(context.Background(), headPrm) if hasWriteCache { require.Eventually(t, func() bool { diff --git a/pkg/local_object_storage/shard/inhume_test.go b/pkg/local_object_storage/shard/inhume_test.go index 3fa6bc0a3..33d483b5e 100644 --- a/pkg/local_object_storage/shard/inhume_test.go +++ b/pkg/local_object_storage/shard/inhume_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -6,7 +6,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" "github.com/stretchr/testify/require" @@ -37,13 +36,13 @@ func testShardInhume(t *testing.T, hasWriteCache bool) { ts := testutil.GenerateObjectWithCID(cnr) - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) - var inhPrm shard.InhumePrm + var inhPrm InhumePrm inhPrm.SetTarget(object.AddressOf(ts), object.AddressOf(obj)) - var getPrm shard.GetPrm + var getPrm GetPrm getPrm.SetAddress(object.AddressOf(obj)) _, err := sh.Put(context.Background(), putPrm) diff --git a/pkg/local_object_storage/shard/list_test.go b/pkg/local_object_storage/shard/list_test.go index 63e7651c8..33b94ce06 100644 --- a/pkg/local_object_storage/shard/list_test.go +++ b/pkg/local_object_storage/shard/list_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -7,7 +7,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" @@ -31,7 +30,7 @@ func TestShard_List(t *testing.T) { }) } -func testShardList(t *testing.T, sh *shard.Shard) { +func testShardList(t *testing.T, sh *Shard) { const C = 10 const N = 5 @@ -59,7 +58,7 @@ func testShardList(t *testing.T, sh *shard.Shard) { objs[object.AddressOf(obj).EncodeToString()] = 0 mtx.Unlock() - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) diff --git a/pkg/local_object_storage/shard/lock_test.go b/pkg/local_object_storage/shard/lock_test.go index da71c4808..be9a0ba9a 100644 --- a/pkg/local_object_storage/shard/lock_test.go +++ b/pkg/local_object_storage/shard/lock_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -11,7 +11,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/fstree" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" meta "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" @@ -26,13 +25,13 @@ import ( func TestShard_Lock(t *testing.T) { t.Parallel() - var sh *shard.Shard + var sh *Shard rootPath := t.TempDir() - opts := []shard.Option{ - shard.WithID(shard.NewIDFromBytes([]byte{})), - shard.WithLogger(&logger.Logger{Logger: zap.NewNop()}), - shard.WithBlobStorOptions( + opts := []Option{ + WithID(NewIDFromBytes([]byte{})), + WithLogger(&logger.Logger{Logger: zap.NewNop()}), + WithBlobStorOptions( blobstor.WithStorages([]blobstor.SubStorage{ { Storage: blobovniczatree.NewBlobovniczaTree( @@ -49,16 +48,16 @@ func TestShard_Lock(t *testing.T) { }, }), ), - shard.WithMetaBaseOptions( + WithMetaBaseOptions( meta.WithPath(filepath.Join(rootPath, "meta")), meta.WithEpochState(epochState{}), ), - shard.WithDeletedLockCallback(func(_ context.Context, addresses []oid.Address) { + WithDeletedLockCallback(func(_ context.Context, addresses []oid.Address) { sh.HandleDeletedLocks(addresses) }), } - sh = shard.New(opts...) + sh = New(opts...) require.NoError(t, sh.Open()) require.NoError(t, sh.Init(context.Background())) @@ -76,7 +75,7 @@ func TestShard_Lock(t *testing.T) { // put the object - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) @@ -94,7 +93,7 @@ func TestShard_Lock(t *testing.T) { t.Run("inhuming locked objects", func(t *testing.T) { ts := testutil.GenerateObjectWithCID(cnr) - var inhumePrm shard.InhumePrm + var inhumePrm InhumePrm inhumePrm.SetTarget(objectcore.AddressOf(ts), objectcore.AddressOf(obj)) var objLockedErr *apistatus.ObjectLocked @@ -110,7 +109,7 @@ func TestShard_Lock(t *testing.T) { t.Run("inhuming lock objects", func(t *testing.T) { ts := testutil.GenerateObjectWithCID(cnr) - var inhumePrm shard.InhumePrm + var inhumePrm InhumePrm inhumePrm.SetTarget(objectcore.AddressOf(ts), objectcore.AddressOf(lock)) _, err = sh.Inhume(context.Background(), inhumePrm) @@ -122,7 +121,7 @@ func TestShard_Lock(t *testing.T) { }) t.Run("force objects inhuming", func(t *testing.T) { - var inhumePrm shard.InhumePrm + var inhumePrm InhumePrm inhumePrm.MarkAsGarbage(objectcore.AddressOf(lock)) inhumePrm.ForceRemoval() @@ -132,7 +131,7 @@ func TestShard_Lock(t *testing.T) { // it should be possible to remove // lock object now - inhumePrm = shard.InhumePrm{} + inhumePrm = InhumePrm{} inhumePrm.MarkAsGarbage(objectcore.AddressOf(obj)) _, err = sh.Inhume(context.Background(), inhumePrm) @@ -140,7 +139,7 @@ func TestShard_Lock(t *testing.T) { // check that object has been removed - var getPrm shard.GetPrm + var getPrm GetPrm getPrm.SetAddress(objectcore.AddressOf(obj)) _, err = sh.Get(context.Background(), getPrm) @@ -160,7 +159,7 @@ func TestShard_IsLocked(t *testing.T) { // put the object - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) diff --git a/pkg/local_object_storage/shard/metrics_test.go b/pkg/local_object_storage/shard/metrics_test.go index 280741f7b..ae993c79c 100644 --- a/pkg/local_object_storage/shard/metrics_test.go +++ b/pkg/local_object_storage/shard/metrics_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -12,7 +12,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" meta "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/pilorama" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard/mode" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" @@ -126,9 +125,6 @@ func (m *metricsStore) DeleteShardMetrics() { m.errCounter = 0 } -const physical = "phy" -const logical = "logic" - func TestCounters(t *testing.T) { t.Parallel() @@ -163,7 +159,7 @@ func TestCounters(t *testing.T) { totalPayload += oSize } - var prm shard.PutPrm + var prm PutPrm for i := 0; i < objNumber; i++ { prm.SetObject(oo[i]) @@ -178,7 +174,7 @@ func TestCounters(t *testing.T) { require.Equal(t, totalPayload, mm.payloadSize()) t.Run("inhume_GC", func(t *testing.T) { - var prm shard.InhumePrm + var prm InhumePrm inhumedNumber := objNumber / 4 for i := 0; i < inhumedNumber; i++ { @@ -197,7 +193,7 @@ func TestCounters(t *testing.T) { }) t.Run("inhume_TS", func(t *testing.T) { - var prm shard.InhumePrm + var prm InhumePrm ts := objectcore.AddressOf(testutil.GenerateObject()) phy := mm.getObjectCounter(physical) @@ -218,7 +214,7 @@ func TestCounters(t *testing.T) { }) t.Run("Delete", func(t *testing.T) { - var prm shard.DeletePrm + var prm DeletePrm phy := mm.getObjectCounter(physical) logic := mm.getObjectCounter(logical) @@ -244,7 +240,7 @@ func TestCounters(t *testing.T) { }) } -func shardWithMetrics(t *testing.T, path string) (*shard.Shard, *metricsStore) { +func shardWithMetrics(t *testing.T, path string) (*Shard, *metricsStore) { blobOpts := []blobstor.Option{ blobstor.WithStorages([]blobstor.SubStorage{ { @@ -264,14 +260,14 @@ func shardWithMetrics(t *testing.T, path string) (*shard.Shard, *metricsStore) { cnrSize: make(map[string]int64), } - sh := shard.New( - shard.WithID(shard.NewIDFromBytes([]byte{})), - shard.WithBlobStorOptions(blobOpts...), - shard.WithPiloramaOptions(pilorama.WithPath(filepath.Join(path, "pilorama"))), - shard.WithMetaBaseOptions( + sh := New( + WithID(NewIDFromBytes([]byte{})), + WithBlobStorOptions(blobOpts...), + WithPiloramaOptions(pilorama.WithPath(filepath.Join(path, "pilorama"))), + WithMetaBaseOptions( meta.WithPath(filepath.Join(path, "meta")), meta.WithEpochState(epochState{})), - shard.WithMetricsWriter(mm), + WithMetricsWriter(mm), ) require.NoError(t, sh.Open()) require.NoError(t, sh.Init(context.Background())) diff --git a/pkg/local_object_storage/shard/range_test.go b/pkg/local_object_storage/shard/range_test.go index 68693f769..4741e5163 100644 --- a/pkg/local_object_storage/shard/range_test.go +++ b/pkg/local_object_storage/shard/range_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -11,7 +11,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/blobovniczatree" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/fstree" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" writecacheconfig "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/config" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebbolt" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger/test" @@ -106,13 +105,13 @@ func testShardGetRange(t *testing.T, hasWriteCache bool) { addr := object.AddressOf(obj) payload := slice.Copy(obj.Payload()) - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) require.NoError(t, err) - var rngPrm shard.RngPrm + var rngPrm RngPrm rngPrm.SetAddress(addr) rngPrm.SetRange(tc.rng.GetOffset(), tc.rng.GetLength()) diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index 6337b0b6e..d1bfc0880 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -11,7 +11,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/fstree" meta "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/pilorama" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" writecacheconfig "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/config" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebbolt" @@ -31,15 +30,15 @@ func (s epochState) CurrentEpoch() uint64 { return s.Value } -func newShard(t testing.TB, enableWriteCache bool) *shard.Shard { +func newShard(t testing.TB, enableWriteCache bool) *Shard { return newCustomShard(t, t.TempDir(), enableWriteCache, writecacheconfig.Options{Type: writecacheconfig.TypeBBolt}, nil, nil) } -func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts writecacheconfig.Options, bsOpts []blobstor.Option, metaOptions []meta.Option) *shard.Shard { - var sh *shard.Shard +func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts writecacheconfig.Options, bsOpts []blobstor.Option, metaOptions []meta.Option) *Shard { + var sh *Shard if enableWriteCache { rootPath = filepath.Join(rootPath, "wc") switch wcOpts.Type { @@ -78,33 +77,33 @@ func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts } } - opts := []shard.Option{ - shard.WithID(shard.NewIDFromBytes([]byte{})), - shard.WithLogger(test.NewLogger(t, true)), - shard.WithBlobStorOptions(bsOpts...), - shard.WithMetaBaseOptions( + opts := []Option{ + WithID(NewIDFromBytes([]byte{})), + WithLogger(test.NewLogger(t, true)), + WithBlobStorOptions(bsOpts...), + WithMetaBaseOptions( append([]meta.Option{ meta.WithPath(filepath.Join(rootPath, "meta")), meta.WithEpochState(epochState{})}, metaOptions...)..., ), - shard.WithPiloramaOptions(pilorama.WithPath(filepath.Join(rootPath, "pilorama"))), - shard.WithWriteCache(enableWriteCache), - shard.WithWriteCacheOptions(wcOpts), - shard.WithDeletedLockCallback(func(_ context.Context, addresses []oid.Address) { + WithPiloramaOptions(pilorama.WithPath(filepath.Join(rootPath, "pilorama"))), + WithWriteCache(enableWriteCache), + WithWriteCacheOptions(wcOpts), + WithDeletedLockCallback(func(_ context.Context, addresses []oid.Address) { sh.HandleDeletedLocks(addresses) }), - shard.WithExpiredLocksCallback(func(ctx context.Context, epoch uint64, a []oid.Address) { + WithExpiredLocksCallback(func(ctx context.Context, epoch uint64, a []oid.Address) { sh.HandleExpiredLocks(ctx, epoch, a) }), - shard.WithGCWorkerPoolInitializer(func(sz int) util.WorkerPool { + WithGCWorkerPoolInitializer(func(sz int) util.WorkerPool { pool, err := ants.NewPool(sz) require.NoError(t, err) return pool }), - shard.WithGCRemoverSleepInterval(100 * time.Millisecond), + WithGCRemoverSleepInterval(100 * time.Millisecond), } - sh = shard.New(opts...) + sh = New(opts...) require.NoError(t, sh.Open()) require.NoError(t, sh.Init(context.Background())) @@ -112,6 +111,6 @@ func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts return sh } -func releaseShard(s *shard.Shard, t testing.TB) { +func releaseShard(s *Shard, t testing.TB) { require.NoError(t, s.Close()) } diff --git a/pkg/local_object_storage/shard/shutdown_test.go b/pkg/local_object_storage/shard/shutdown_test.go index 68ef90963..304448b1b 100644 --- a/pkg/local_object_storage/shard/shutdown_test.go +++ b/pkg/local_object_storage/shard/shutdown_test.go @@ -1,4 +1,4 @@ -package shard_test +package shard import ( "context" @@ -7,7 +7,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/shard" writecacheconfig "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/config" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebbolt" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" @@ -50,7 +49,7 @@ func TestWriteCacheObjectLoss(t *testing.T) { for i := range objects { obj := objects[i] errG.Go(func() error { - var putPrm shard.PutPrm + var putPrm PutPrm putPrm.SetObject(obj) _, err := sh.Put(context.Background(), putPrm) return err @@ -62,7 +61,7 @@ func TestWriteCacheObjectLoss(t *testing.T) { sh = newCustomShard(t, dir, true, wcOpts, nil, nil) defer releaseShard(sh, t) - var getPrm shard.GetPrm + var getPrm GetPrm for i := range objects { getPrm.SetAddress(object.AddressOf(objects[i])) -- 2.45.3 From b7694b6f45dce52cc1944f3816161da877ba72a1 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 31 Aug 2023 16:33:37 +0300 Subject: [PATCH 04/11] [#668] shard: Close stopChannel in GC It is done once, but now we could read it from multiple places. Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/local_object_storage/shard/gc.go b/pkg/local_object_storage/shard/gc.go index 2221d57c1..8be91fcaf 100644 --- a/pkg/local_object_storage/shard/gc.go +++ b/pkg/local_object_storage/shard/gc.go @@ -220,7 +220,7 @@ func (gc *gc) tickRemover(ctx context.Context) { func (gc *gc) stop() { gc.onceStop.Do(func() { - gc.stopChannel <- struct{}{} + close(gc.stopChannel) }) gc.log.Info(logs.ShardWaitingForGCWorkersToStop) -- 2.45.3 From bce4210119f282c5681d16c87ec264a2e0be095c Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 31 Aug 2023 18:17:55 +0300 Subject: [PATCH 05/11] [#668] shard/test: Disable GC where it is not needed Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc.go | 9 ++++++++- pkg/local_object_storage/shard/gc_internal_test.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/local_object_storage/shard/gc.go b/pkg/local_object_storage/shard/gc.go index 8be91fcaf..38baccd4b 100644 --- a/pkg/local_object_storage/shard/gc.go +++ b/pkg/local_object_storage/shard/gc.go @@ -119,6 +119,8 @@ type gcCfg struct { expiredCollectorBatchSize int metrics GCMectrics + + testHookRemover func(ctx context.Context) gcRunResult } func defaultGCCfg() gcCfg { @@ -209,7 +211,12 @@ func (gc *gc) tickRemover(ctx context.Context) { case <-timer.C: startedAt := time.Now() - result := gc.remover(ctx) + var result gcRunResult + if gc.testHookRemover != nil { + result = gc.testHookRemover(ctx) + } else { + result = gc.remover(ctx) + } timer.Reset(gc.removerInterval) gc.metrics.AddRunDuration(time.Since(startedAt), result.success) diff --git a/pkg/local_object_storage/shard/gc_internal_test.go b/pkg/local_object_storage/shard/gc_internal_test.go index 9a4f52029..c8925e01f 100644 --- a/pkg/local_object_storage/shard/gc_internal_test.go +++ b/pkg/local_object_storage/shard/gc_internal_test.go @@ -75,7 +75,7 @@ func Test_ObjectNotFoundIfNotDeletedFromMetabase(t *testing.T) { } sh = New(opts...) - + sh.gcCfg.testHookRemover = func(context.Context) gcRunResult { return gcRunResult{} } require.NoError(t, sh.Open()) require.NoError(t, sh.Init(context.Background())) -- 2.45.3 From a2c5a0581c5d2a39a448e8f533e5de1564fe4eec Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 17:18:04 +0300 Subject: [PATCH 06/11] [#668] shard/test: Release shard in t.Cleanup() Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/delete_test.go | 1 - pkg/local_object_storage/shard/get_test.go | 1 - pkg/local_object_storage/shard/head_test.go | 1 - pkg/local_object_storage/shard/inhume_test.go | 1 - pkg/local_object_storage/shard/list_test.go | 2 -- pkg/local_object_storage/shard/shard_test.go | 4 +++- 6 files changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/local_object_storage/shard/delete_test.go b/pkg/local_object_storage/shard/delete_test.go index 8ee613186..3421ac9e0 100644 --- a/pkg/local_object_storage/shard/delete_test.go +++ b/pkg/local_object_storage/shard/delete_test.go @@ -28,7 +28,6 @@ func TestShard_Delete(t *testing.T) { func testShardDelete(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) - defer releaseShard(sh, t) cnr := cidtest.ID() diff --git a/pkg/local_object_storage/shard/get_test.go b/pkg/local_object_storage/shard/get_test.go index d3aeb5339..19a5e8d70 100644 --- a/pkg/local_object_storage/shard/get_test.go +++ b/pkg/local_object_storage/shard/get_test.go @@ -32,7 +32,6 @@ func TestShard_Get(t *testing.T) { func testShardGet(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) - defer releaseShard(sh, t) var putPrm PutPrm var getPrm GetPrm diff --git a/pkg/local_object_storage/shard/head_test.go b/pkg/local_object_storage/shard/head_test.go index bf6ba990b..dfae48e84 100644 --- a/pkg/local_object_storage/shard/head_test.go +++ b/pkg/local_object_storage/shard/head_test.go @@ -30,7 +30,6 @@ func TestShard_Head(t *testing.T) { func testShardHead(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) - defer releaseShard(sh, t) var putPrm PutPrm var headPrm HeadPrm diff --git a/pkg/local_object_storage/shard/inhume_test.go b/pkg/local_object_storage/shard/inhume_test.go index 33d483b5e..6c8e46faf 100644 --- a/pkg/local_object_storage/shard/inhume_test.go +++ b/pkg/local_object_storage/shard/inhume_test.go @@ -27,7 +27,6 @@ func TestShard_Inhume(t *testing.T) { func testShardInhume(t *testing.T, hasWriteCache bool) { sh := newShard(t, hasWriteCache) - defer releaseShard(sh, t) cnr := cidtest.ID() diff --git a/pkg/local_object_storage/shard/list_test.go b/pkg/local_object_storage/shard/list_test.go index 33b94ce06..9ca1753c4 100644 --- a/pkg/local_object_storage/shard/list_test.go +++ b/pkg/local_object_storage/shard/list_test.go @@ -18,14 +18,12 @@ func TestShard_List(t *testing.T) { t.Run("without write cache", func(t *testing.T) { t.Parallel() sh := newShard(t, false) - defer releaseShard(sh, t) testShardList(t, sh) }) t.Run("with write cache", func(t *testing.T) { t.Parallel() shWC := newShard(t, true) - defer releaseShard(shWC, t) testShardList(t, shWC) }) } diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index d1bfc0880..1550af5c0 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -31,10 +31,12 @@ func (s epochState) CurrentEpoch() uint64 { } func newShard(t testing.TB, enableWriteCache bool) *Shard { - return newCustomShard(t, t.TempDir(), enableWriteCache, + sh := newCustomShard(t, t.TempDir(), enableWriteCache, writecacheconfig.Options{Type: writecacheconfig.TypeBBolt}, nil, nil) + t.Cleanup(func() { releaseShard(sh, t) }) + return sh } func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts writecacheconfig.Options, bsOpts []blobstor.Option, metaOptions []meta.Option) *Shard { -- 2.45.3 From 673893722f5225768c6c4b64ee2333244ec22ef8 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 17:32:14 +0300 Subject: [PATCH 07/11] [#668] shard/test: Simplify shard construction newCustomShard() has many parameters but only the first is obligatory. `enableWriteCache` is left as-is, because it directly affects the functionality. Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc_test.go | 4 +- pkg/local_object_storage/shard/range_test.go | 38 +++++++------ pkg/local_object_storage/shard/shard_test.go | 53 +++++++++++-------- .../shard/shutdown_test.go | 4 +- 4 files changed, 55 insertions(+), 44 deletions(-) diff --git a/pkg/local_object_storage/shard/gc_test.go b/pkg/local_object_storage/shard/gc_test.go index 24038f10b..a7daadac8 100644 --- a/pkg/local_object_storage/shard/gc_test.go +++ b/pkg/local_object_storage/shard/gc_test.go @@ -29,7 +29,7 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { wcOpts := writecacheconfig.Options{ Type: writecacheconfig.TypeBBolt, } - sh := newCustomShard(t, t.TempDir(), false, wcOpts, nil, []meta.Option{meta.WithEpochState(epoch)}) + sh := newCustomShard(t, false, shardOptions{rootPath: t.TempDir(), wcOpts: wcOpts, metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) t.Cleanup(func() { releaseShard(sh, t) @@ -129,7 +129,7 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { wcOpts := writecacheconfig.Options{ Type: writecacheconfig.TypeBBolt, } - sh := newCustomShard(t, t.TempDir(), false, wcOpts, nil, []meta.Option{meta.WithEpochState(epoch)}) + sh := newCustomShard(t, false, shardOptions{rootPath: t.TempDir(), wcOpts: wcOpts, metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) t.Cleanup(func() { releaseShard(sh, t) diff --git a/pkg/local_object_storage/shard/range_test.go b/pkg/local_object_storage/shard/range_test.go index 4741e5163..8c2f400d4 100644 --- a/pkg/local_object_storage/shard/range_test.go +++ b/pkg/local_object_storage/shard/range_test.go @@ -76,24 +76,28 @@ func testShardGetRange(t *testing.T, hasWriteCache bool) { }, } - sh := newCustomShard(t, t.TempDir(), hasWriteCache, wcOpts, - []blobstor.Option{blobstor.WithStorages([]blobstor.SubStorage{ - { - Storage: blobovniczatree.NewBlobovniczaTree( - blobovniczatree.WithLogger(test.NewLogger(t, true)), - blobovniczatree.WithRootPath(filepath.Join(t.TempDir(), "blob", "blobovnicza")), - blobovniczatree.WithBlobovniczaShallowDepth(1), - blobovniczatree.WithBlobovniczaShallowWidth(1)), - Policy: func(_ *objectSDK.Object, data []byte) bool { - return len(data) <= smallObjectSize + sh := newCustomShard(t, hasWriteCache, shardOptions{ + rootPath: t.TempDir(), + wcOpts: wcOpts, + bsOpts: []blobstor.Option{ + blobstor.WithStorages([]blobstor.SubStorage{ + { + Storage: blobovniczatree.NewBlobovniczaTree( + blobovniczatree.WithLogger(test.NewLogger(t, true)), + blobovniczatree.WithRootPath(filepath.Join(t.TempDir(), "blob", "blobovnicza")), + blobovniczatree.WithBlobovniczaShallowDepth(1), + blobovniczatree.WithBlobovniczaShallowWidth(1)), + Policy: func(_ *objectSDK.Object, data []byte) bool { + return len(data) <= smallObjectSize + }, }, - }, - { - Storage: fstree.New( - fstree.WithPath(filepath.Join(t.TempDir(), "blob"))), - }, - })}, - nil) + { + Storage: fstree.New( + fstree.WithPath(filepath.Join(t.TempDir(), "blob"))), + }, + }), + }, + }) defer releaseShard(sh, t) for _, tc := range testCases { diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index 1550af5c0..1318bd28f 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -30,41 +30,48 @@ func (s epochState) CurrentEpoch() uint64 { return s.Value } +type shardOptions struct { + rootPath string + wcOpts writecacheconfig.Options + bsOpts []blobstor.Option + metaOptions []meta.Option +} + func newShard(t testing.TB, enableWriteCache bool) *Shard { - sh := newCustomShard(t, t.TempDir(), enableWriteCache, - writecacheconfig.Options{Type: writecacheconfig.TypeBBolt}, - nil, - nil) + sh := newCustomShard(t, enableWriteCache, shardOptions{ + rootPath: t.TempDir(), + wcOpts: writecacheconfig.Options{Type: writecacheconfig.TypeBBolt}, + }) t.Cleanup(func() { releaseShard(sh, t) }) return sh } -func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts writecacheconfig.Options, bsOpts []blobstor.Option, metaOptions []meta.Option) *Shard { +func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard { var sh *Shard if enableWriteCache { - rootPath = filepath.Join(rootPath, "wc") - switch wcOpts.Type { + o.rootPath = filepath.Join(o.rootPath, "wc") + switch o.wcOpts.Type { case writecacheconfig.TypeBBolt: - wcOpts.BBoltOptions = append( - []writecachebbolt.Option{writecachebbolt.WithPath(filepath.Join(rootPath, "wcache"))}, - wcOpts.BBoltOptions...) + o.wcOpts.BBoltOptions = append( + []writecachebbolt.Option{writecachebbolt.WithPath(filepath.Join(o.rootPath, "wcache"))}, + o.wcOpts.BBoltOptions...) case writecacheconfig.TypeBadger: - wcOpts.BadgerOptions = append( - []writecachebadger.Option{writecachebadger.WithPath(filepath.Join(rootPath, "wcache"))}, - wcOpts.BadgerOptions...) + o.wcOpts.BadgerOptions = append( + []writecachebadger.Option{writecachebadger.WithPath(filepath.Join(o.rootPath, "wcache"))}, + o.wcOpts.BadgerOptions...) } } else { - rootPath = filepath.Join(rootPath, "nowc") + o.rootPath = filepath.Join(o.rootPath, "nowc") } - if bsOpts == nil { - bsOpts = []blobstor.Option{ + if o.bsOpts == nil { + o.bsOpts = []blobstor.Option{ blobstor.WithLogger(test.NewLogger(t, true)), blobstor.WithStorages([]blobstor.SubStorage{ { Storage: blobovniczatree.NewBlobovniczaTree( blobovniczatree.WithLogger(test.NewLogger(t, true)), - blobovniczatree.WithRootPath(filepath.Join(rootPath, "blob", "blobovnicza")), + blobovniczatree.WithRootPath(filepath.Join(o.rootPath, "blob", "blobovnicza")), blobovniczatree.WithBlobovniczaShallowDepth(1), blobovniczatree.WithBlobovniczaShallowWidth(1)), Policy: func(_ *objectSDK.Object, data []byte) bool { @@ -73,7 +80,7 @@ func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts }, { Storage: fstree.New( - fstree.WithPath(filepath.Join(rootPath, "blob"))), + fstree.WithPath(filepath.Join(o.rootPath, "blob"))), }, }), } @@ -82,15 +89,15 @@ func newCustomShard(t testing.TB, rootPath string, enableWriteCache bool, wcOpts opts := []Option{ WithID(NewIDFromBytes([]byte{})), WithLogger(test.NewLogger(t, true)), - WithBlobStorOptions(bsOpts...), + WithBlobStorOptions(o.bsOpts...), WithMetaBaseOptions( append([]meta.Option{ - meta.WithPath(filepath.Join(rootPath, "meta")), meta.WithEpochState(epochState{})}, - metaOptions...)..., + meta.WithPath(filepath.Join(o.rootPath, "meta")), meta.WithEpochState(epochState{})}, + o.metaOptions...)..., ), - WithPiloramaOptions(pilorama.WithPath(filepath.Join(rootPath, "pilorama"))), + WithPiloramaOptions(pilorama.WithPath(filepath.Join(o.rootPath, "pilorama"))), WithWriteCache(enableWriteCache), - WithWriteCacheOptions(wcOpts), + WithWriteCacheOptions(o.wcOpts), WithDeletedLockCallback(func(_ context.Context, addresses []oid.Address) { sh.HandleDeletedLocks(addresses) }), diff --git a/pkg/local_object_storage/shard/shutdown_test.go b/pkg/local_object_storage/shard/shutdown_test.go index 304448b1b..6cfa5fb6b 100644 --- a/pkg/local_object_storage/shard/shutdown_test.go +++ b/pkg/local_object_storage/shard/shutdown_test.go @@ -43,7 +43,7 @@ func TestWriteCacheObjectLoss(t *testing.T) { }, } - sh := newCustomShard(t, dir, true, wcOpts, nil, nil) + sh := newCustomShard(t, true, shardOptions{rootPath: dir, wcOpts: wcOpts}) var errG errgroup.Group for i := range objects { @@ -58,7 +58,7 @@ func TestWriteCacheObjectLoss(t *testing.T) { require.NoError(t, errG.Wait()) require.NoError(t, sh.Close()) - sh = newCustomShard(t, dir, true, wcOpts, nil, nil) + sh = newCustomShard(t, true, shardOptions{rootPath: dir, wcOpts: wcOpts}) defer releaseShard(sh, t) var getPrm GetPrm -- 2.45.3 From 500128bf40fa337dbf54c79c8db52ec2510d968f Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 17:48:00 +0300 Subject: [PATCH 08/11] [#668] shard/test: Use sane defaults in the test constructor Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc_test.go | 11 ++--------- pkg/local_object_storage/shard/range_test.go | 3 +-- pkg/local_object_storage/shard/shard_test.go | 12 ++++++++---- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/local_object_storage/shard/gc_test.go b/pkg/local_object_storage/shard/gc_test.go index a7daadac8..d6a7e39e2 100644 --- a/pkg/local_object_storage/shard/gc_test.go +++ b/pkg/local_object_storage/shard/gc_test.go @@ -10,7 +10,6 @@ import ( objectCore "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" - writecacheconfig "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/config" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -26,10 +25,7 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { Value: 100, } - wcOpts := writecacheconfig.Options{ - Type: writecacheconfig.TypeBBolt, - } - sh := newCustomShard(t, false, shardOptions{rootPath: t.TempDir(), wcOpts: wcOpts, metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) + sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) t.Cleanup(func() { releaseShard(sh, t) @@ -126,10 +122,7 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { linkID, _ := link.ID() - wcOpts := writecacheconfig.Options{ - Type: writecacheconfig.TypeBBolt, - } - sh := newCustomShard(t, false, shardOptions{rootPath: t.TempDir(), wcOpts: wcOpts, metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) + sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) t.Cleanup(func() { releaseShard(sh, t) diff --git a/pkg/local_object_storage/shard/range_test.go b/pkg/local_object_storage/shard/range_test.go index 8c2f400d4..77a20966a 100644 --- a/pkg/local_object_storage/shard/range_test.go +++ b/pkg/local_object_storage/shard/range_test.go @@ -77,8 +77,7 @@ func testShardGetRange(t *testing.T, hasWriteCache bool) { } sh := newCustomShard(t, hasWriteCache, shardOptions{ - rootPath: t.TempDir(), - wcOpts: wcOpts, + wcOpts: wcOpts, bsOpts: []blobstor.Option{ blobstor.WithStorages([]blobstor.SubStorage{ { diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index 1318bd28f..33560407e 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -38,15 +38,19 @@ type shardOptions struct { } func newShard(t testing.TB, enableWriteCache bool) *Shard { - sh := newCustomShard(t, enableWriteCache, shardOptions{ - rootPath: t.TempDir(), - wcOpts: writecacheconfig.Options{Type: writecacheconfig.TypeBBolt}, - }) + sh := newCustomShard(t, enableWriteCache, shardOptions{}) t.Cleanup(func() { releaseShard(sh, t) }) return sh } func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard { + if o.rootPath == "" { + o.rootPath = t.TempDir() + } + if enableWriteCache && o.wcOpts.Type == 0 { + o.wcOpts.Type = writecacheconfig.TypeBBolt + } + var sh *Shard if enableWriteCache { o.rootPath = filepath.Join(o.rootPath, "wc") -- 2.45.3 From 73e02b3d8214787802219f16688274f4e176d71f Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 17:51:15 +0300 Subject: [PATCH 09/11] [#668] shard/test: Add dontRelease options Most of the time we would like to close shard with minor exceptions. Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc_test.go | 8 -------- pkg/local_object_storage/shard/range_test.go | 1 - pkg/local_object_storage/shard/shard_test.go | 9 ++++++--- pkg/local_object_storage/shard/shutdown_test.go | 3 +-- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/local_object_storage/shard/gc_test.go b/pkg/local_object_storage/shard/gc_test.go index d6a7e39e2..4f19db878 100644 --- a/pkg/local_object_storage/shard/gc_test.go +++ b/pkg/local_object_storage/shard/gc_test.go @@ -27,10 +27,6 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) - t.Cleanup(func() { - releaseShard(sh, t) - }) - cnr := cidtest.ID() var objExpirationAttr objectSDK.Attribute @@ -124,10 +120,6 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) - t.Cleanup(func() { - releaseShard(sh, t) - }) - lock := testutil.GenerateObjectWithCID(cnr) lock.SetType(objectSDK.TypeLock) lock.SetAttributes(lockExpirationAttr) diff --git a/pkg/local_object_storage/shard/range_test.go b/pkg/local_object_storage/shard/range_test.go index 77a20966a..a8bc83307 100644 --- a/pkg/local_object_storage/shard/range_test.go +++ b/pkg/local_object_storage/shard/range_test.go @@ -97,7 +97,6 @@ func testShardGetRange(t *testing.T, hasWriteCache bool) { }), }, }) - defer releaseShard(sh, t) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index 33560407e..eb2aba580 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -32,15 +32,14 @@ func (s epochState) CurrentEpoch() uint64 { type shardOptions struct { rootPath string + dontRelease bool wcOpts writecacheconfig.Options bsOpts []blobstor.Option metaOptions []meta.Option } func newShard(t testing.TB, enableWriteCache bool) *Shard { - sh := newCustomShard(t, enableWriteCache, shardOptions{}) - t.Cleanup(func() { releaseShard(sh, t) }) - return sh + return newCustomShard(t, enableWriteCache, shardOptions{}) } func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard { @@ -121,6 +120,10 @@ func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard require.NoError(t, sh.Open()) require.NoError(t, sh.Init(context.Background())) + if !o.dontRelease { + t.Cleanup(func() { releaseShard(sh, t) }) + } + return sh } diff --git a/pkg/local_object_storage/shard/shutdown_test.go b/pkg/local_object_storage/shard/shutdown_test.go index 6cfa5fb6b..163c3a4ae 100644 --- a/pkg/local_object_storage/shard/shutdown_test.go +++ b/pkg/local_object_storage/shard/shutdown_test.go @@ -43,7 +43,7 @@ func TestWriteCacheObjectLoss(t *testing.T) { }, } - sh := newCustomShard(t, true, shardOptions{rootPath: dir, wcOpts: wcOpts}) + sh := newCustomShard(t, true, shardOptions{dontRelease: true, rootPath: dir, wcOpts: wcOpts}) var errG errgroup.Group for i := range objects { @@ -59,7 +59,6 @@ func TestWriteCacheObjectLoss(t *testing.T) { require.NoError(t, sh.Close()) sh = newCustomShard(t, true, shardOptions{rootPath: dir, wcOpts: wcOpts}) - defer releaseShard(sh, t) var getPrm GetPrm -- 2.45.3 From 1c17a8a06e5467cd3c6e3dbb471eb2eac44f0b06 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 18:03:04 +0300 Subject: [PATCH 10/11] [#668] shard/test: Properly check event processing See https://git.frostfs.info/TrueCloudLab/frostfs-node/actions/runs/1594/jobs/2 Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/gc.go | 46 +++++++++++--------- pkg/local_object_storage/shard/gc_test.go | 32 ++++++++------ pkg/local_object_storage/shard/shard_test.go | 3 ++ 3 files changed, 47 insertions(+), 34 deletions(-) diff --git a/pkg/local_object_storage/shard/gc.go b/pkg/local_object_storage/shard/gc.go index 38baccd4b..13ab39ae0 100644 --- a/pkg/local_object_storage/shard/gc.go +++ b/pkg/local_object_storage/shard/gc.go @@ -160,33 +160,37 @@ func (gc *gc) listenEvents(ctx context.Context) { return } - v, ok := gc.mEventHandler[event.typ()] - if !ok { - continue - } + gc.handleEvent(ctx, event) + } +} - v.cancelFunc() - v.prevGroup.Wait() +func (gc *gc) handleEvent(ctx context.Context, event Event) { + v, ok := gc.mEventHandler[event.typ()] + if !ok { + return + } - var runCtx context.Context - runCtx, v.cancelFunc = context.WithCancel(ctx) + v.cancelFunc() + v.prevGroup.Wait() - v.prevGroup.Add(len(v.handlers)) + var runCtx context.Context + runCtx, v.cancelFunc = context.WithCancel(ctx) - for i := range v.handlers { - h := v.handlers[i] + v.prevGroup.Add(len(v.handlers)) - err := gc.workerPool.Submit(func() { - defer v.prevGroup.Done() - h(runCtx, event) - }) - if err != nil { - gc.log.Warn(logs.ShardCouldNotSubmitGCJobToWorkerPool, - zap.String("error", err.Error()), - ) + for i := range v.handlers { + h := v.handlers[i] - v.prevGroup.Done() - } + err := gc.workerPool.Submit(func() { + defer v.prevGroup.Done() + h(runCtx, event) + }) + if err != nil { + gc.log.Warn(logs.ShardCouldNotSubmitGCJobToWorkerPool, + zap.String("error", err.Error()), + ) + + v.prevGroup.Done() } } } diff --git a/pkg/local_object_storage/shard/gc_test.go b/pkg/local_object_storage/shard/gc_test.go index 4f19db878..8b535200d 100644 --- a/pkg/local_object_storage/shard/gc_test.go +++ b/pkg/local_object_storage/shard/gc_test.go @@ -4,12 +4,12 @@ import ( "context" "errors" "testing" - "time" objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" objectCore "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" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" cidtest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id/test" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -25,7 +25,12 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { Value: 100, } - sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) + sh := newCustomShard(t, false, shardOptions{ + metaOptions: []meta.Option{meta.WithEpochState(epoch)}, + additionalShardOptions: []Option{WithGCWorkerPoolInitializer(func(int) util.WorkerPool { + return util.NewPseudoWorkerPool() // synchronous event processing + })}, + }) cnr := cidtest.ID() @@ -60,14 +65,12 @@ func Test_GCDropsLockedExpiredSimpleObject(t *testing.T) { require.NoError(t, err) epoch.Value = 105 - sh.NotificationChannel() <- EventNewEpoch(epoch.Value) + sh.gc.handleEvent(context.Background(), EventNewEpoch(epoch.Value)) var getPrm GetPrm getPrm.SetAddress(objectCore.AddressOf(obj)) - require.Eventually(t, func() bool { - _, err = sh.Get(context.Background(), getPrm) - return client.IsErrObjectNotFound(err) - }, 3*time.Second, 1*time.Second, "expired object must be deleted") + _, err = sh.Get(context.Background(), getPrm) + require.True(t, client.IsErrObjectNotFound(err), "expired object must be deleted") } func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { @@ -118,7 +121,12 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { linkID, _ := link.ID() - sh := newCustomShard(t, false, shardOptions{metaOptions: []meta.Option{meta.WithEpochState(epoch)}}) + sh := newCustomShard(t, false, shardOptions{ + metaOptions: []meta.Option{meta.WithEpochState(epoch)}, + additionalShardOptions: []Option{WithGCWorkerPoolInitializer(func(int) util.WorkerPool { + return util.NewPseudoWorkerPool() // synchronous event processing + })}, + }) lock := testutil.GenerateObjectWithCID(cnr) lock.SetType(objectSDK.TypeLock) @@ -152,10 +160,8 @@ func Test_GCDropsLockedExpiredComplexObject(t *testing.T) { require.True(t, errors.As(err, &splitInfoError), "split info must be provided") epoch.Value = 105 - sh.NotificationChannel() <- EventNewEpoch(epoch.Value) + sh.gc.handleEvent(context.Background(), EventNewEpoch(epoch.Value)) - require.Eventually(t, func() bool { - _, err = sh.Get(context.Background(), getPrm) - return client.IsErrObjectNotFound(err) - }, 3*time.Second, 1*time.Second, "expired complex object must be deleted on epoch after lock expires") + _, err = sh.Get(context.Background(), getPrm) + require.True(t, client.IsErrObjectNotFound(err), "expired complex object must be deleted on epoch after lock expires") } diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index eb2aba580..669ac287c 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -36,6 +36,8 @@ type shardOptions struct { wcOpts writecacheconfig.Options bsOpts []blobstor.Option metaOptions []meta.Option + + additionalShardOptions []Option } func newShard(t testing.TB, enableWriteCache bool) *Shard { @@ -114,6 +116,7 @@ func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard }), WithGCRemoverSleepInterval(100 * time.Millisecond), } + opts = append(opts, o.additionalShardOptions...) sh = New(opts...) -- 2.45.3 From d3ede98cd9799fdbc5841c1e9efa30fa5c47a409 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 6 Sep 2023 18:08:23 +0300 Subject: [PATCH 11/11] [#668] shard/test: Do not alter rootPath option Supposedly, this was added to allow creating 2 different shards without subtest. Now we use t.TempDir() everywhere, so this should not be a problem. Signed-off-by: Evgenii Stratonikov --- pkg/local_object_storage/shard/shard_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/local_object_storage/shard/shard_test.go b/pkg/local_object_storage/shard/shard_test.go index 669ac287c..5dcccd9c9 100644 --- a/pkg/local_object_storage/shard/shard_test.go +++ b/pkg/local_object_storage/shard/shard_test.go @@ -54,7 +54,6 @@ func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard var sh *Shard if enableWriteCache { - o.rootPath = filepath.Join(o.rootPath, "wc") switch o.wcOpts.Type { case writecacheconfig.TypeBBolt: o.wcOpts.BBoltOptions = append( @@ -65,8 +64,6 @@ func newCustomShard(t testing.TB, enableWriteCache bool, o shardOptions) *Shard []writecachebadger.Option{writecachebadger.WithPath(filepath.Join(o.rootPath, "wcache"))}, o.wcOpts.BadgerOptions...) } - } else { - o.rootPath = filepath.Join(o.rootPath, "nowc") } if o.bsOpts == nil { -- 2.45.3