[#xx] Add general sanity tests for object stores #671

Closed
ale64bit wants to merge 1 commit from ale64bit/frostfs-node:feature/store-basic-sanity-test into master

View file

@ -0,0 +1,182 @@
package storagetest
import (
"testing"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/internal/testutil"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client"
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"
"golang.org/x/exp/rand"
)
// ObjectStore is the general interface of implementations of object storage.
//
// It can be used to wrap other storages and execute generic sanity tests on them.
type ObjectStore interface {
Review

Where is it intended to be used? Wouldn't it be better to put in blobstor/internal/blobstortest?

Where is it intended to be used? Wouldn't it be better to put in `blobstor/internal/blobstortest`?
Review

Not sure there's an actual difference for us between a "blob" and an "object" from the storage viewpoint. Why to move it there?

Anyway, it's intended to be used everywhere it can be plugged in. For example:

  • whole engine can be wrapped to implement ObjectStore.
  • whole shard can be wrapped to implement ObjectStore.
  • any of the common.Storage implementations can be wrapped and used almost verbatim.
  • any writecache implementation can be wrapped in a shard, using the memstore as blobstor and metabase, to test that the interaction between writecache and backing storage is correct.
Not sure there's an actual difference for us between a "blob" and an "object" from the storage viewpoint. Why to move it there? Anyway, it's intended to be used everywhere it can be plugged in. For example: - whole engine can be wrapped to implement `ObjectStore`. - whole shard can be wrapped to implement `ObjectStore`. - any of the `common.Storage` implementations can be wrapped and used almost verbatim. - any writecache implementation can be wrapped in a shard, using the `memstore` as blobstor and metabase, to test that the interaction between writecache and backing storage is correct.
Review

I would like these generic tests suites to have specific purpose, not scattered around the codebase.
Does it make sense to use these tests for common.Storage or they are already covered in another suite?
Why not just use common.Storage as an interface and store somewhere close to blobstortest?

I would like these generic tests suites to have specific purpose, not scattered around the codebase. Does it make sense to use these tests for common.Storage or they are already covered in another suite? Why not just use `common.Storage` as an interface and store somewhere close to blobstortest?
Review

I would like these generic tests suites to have specific purpose, not scattered around the codebase.

Cool. Note that "generic" means that the purpose might not be "specific".

Does it make sense to use these tests for common.Storage or they are already covered in another suite?

These tests make sense for anything that implements ObjectStore or is conceptually close to it.

Why not just use common.Storage as an interface and store somewhere close to blobstortest?

Because it's bloated as hell.

> I would like these generic tests suites to have specific purpose, not scattered around the codebase. Cool. Note that "generic" means that the purpose might not be "specific". > Does it make sense to use these tests for common.Storage or they are already covered in another suite? These tests make sense for anything that implements `ObjectStore` or is conceptually close to it. > Why not just use common.Storage as an interface and store somewhere close to blobstortest? Because it's bloated as hell.
Review

Cool. Note that "generic" means that the purpose might not be "specific".

Generic over the implementation, specific in functional purpose.

Because it's bloated as hell.

Adding yet another suite of tests (similar in spirit but with different interface) doesn't improve the situation, and likely, makes it even worse.

> Cool. Note that "generic" means that the purpose might not be "specific". Generic over the implementation, specific in functional purpose. > Because it's bloated as hell. Adding yet another suite of tests (similar in spirit but with different interface) doesn't improve the situation, and likely, makes it even worse.
Put(*objectSDK.Object) error
Get(oid.Address) (*objectSDK.Object, error)
Delete(oid.Address) error
}
func TestSanityBasicLifecycle(t *testing.T, st ObjectStore) {
obj := testutil.GenerateObject()
addr := testutil.AddressFromObject(t, obj)
data, err := obj.Marshal()
require.NoError(t, err)
// Get nonexistent object
{
_, gotErr := st.Get(addr)
require.True(t, client.IsErrObjectNotFound(gotErr))
}
// Put an object
require.NoError(t, st.Put(obj))
// Get the object previously put
{
gotObj, err := st.Get(addr)
require.NoError(t, err)
gotData, err := gotObj.Marshal()
require.NoError(t, err)
require.Equal(t, data, gotData)
}
// Delete the object previously put
{
require.NoError(t, st.Delete(addr))
require.True(t, client.IsErrObjectNotFound(st.Delete(addr)))
}
// Get the object previously deleted
{
_, gotErr := st.Get(addr)
require.True(t, client.IsErrObjectNotFound(gotErr))
}
}
func TestSanityPutGetSequence(t *testing.T, st ObjectStore, iterations int) {
var objs []*objectSDK.Object
for i := 0; i < iterations; i++ {
obj := testutil.GenerateObject()
require.NoError(t, st.Put(obj))
objs = append(objs, obj)
}
for _, wantObj := range objs {
addr := testutil.AddressFromObject(t, wantObj)
gotObj, err := st.Get(addr)
require.NoError(t, err)
wantData, err := wantObj.Marshal()
require.NoError(t, err)
gotData, err := gotObj.Marshal()
require.NoError(t, err)
require.Equal(t, wantData, gotData)
}
}
func TestSanityPutDeleteSequence(t *testing.T, st ObjectStore, iterations int) {
for i := 0; i < iterations; i++ {
obj := testutil.GenerateObject()
addr := testutil.AddressFromObject(t, obj)
require.NoError(t, st.Put(obj))
require.NoError(t, st.Delete(addr))
require.True(t, client.IsErrObjectNotFound(st.Delete(addr)))
}
}
func TestSanityStress(t *testing.T, st ObjectStore, iterations, maxResidentObjects int) {
type entry struct {
obj *objectSDK.Object
index int
}
objs := map[oid.Address]*entry{}
var addrs []oid.Address
putOne := func() {
obj := testutil.GenerateObject()
addr := testutil.AddressFromObject(t, obj)
require.NoError(t, st.Put(obj))
objs[addr] = &entry{obj, len(addrs)}
addrs = append(addrs, addr)
}
putOverwrite := func() {
index := rand.Intn(len(addrs))
addr := addrs[index]
require.NoError(t, st.Put(objs[addr].obj))
}
getOne := func() {
index := rand.Intn(len(addrs))
addr := addrs[index]
wantObj := objs[addr].obj
wantData, err := wantObj.Marshal()
require.NoError(t, err)
gotObj, err := st.Get(addr)
require.NoError(t, err)
gotData, err := gotObj.Marshal()
require.NoError(t, err)
require.Equal(t, wantData, gotData)
}
getNonexistent := func() {
addr := randNonexistingAddr(objs)
_, err := st.Get(addr)
require.True(t, client.IsErrObjectNotFound(err))
}
deleteOne := func() {
index := rand.Intn(len(addrs))
addr := addrs[index]
require.NoError(t, st.Delete(addr))
objs[addrs[len(addrs)-1]].index = index
addrs[index] = addrs[len(addrs)-1]
addrs = addrs[:len(addrs)-1]
delete(objs, addr)
}
deleteNonexistent := func() {
addr := randNonexistingAddr(objs)
require.True(t, client.IsErrObjectNotFound(st.Delete(addr)))
}
for i := 0; i < iterations; i++ {
// Make a slice of the operations that are currently possible
validOps := []func(){getNonexistent, deleteNonexistent}
if len(addrs) < maxResidentObjects {
validOps = append(validOps, putOne)
}
if len(addrs) > 0 {
validOps = append(validOps, getOne, putOverwrite, deleteOne)
}
// Run a random operation
validOps[rand.Intn(len(validOps))]()
}
}
func randNonexistingAddr[V any](objs map[oid.Address]V) oid.Address {
addr := oidtest.Address()
// Make sure the key doesn't exist
for {
if _, exists := objs[addr]; !exists {
break
}
addr = oidtest.Address()
}
return addr
}