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

ale64bit · 2023-08-31T14:22:34Z

ale64bit commented

2023-08-31 14:22:34 +00:00

Signed-off-by: Alejandro Lopez a.lopez@yadro.com

Signed-off-by: Alejandro Lopez <a.lopez@yadro.com>

ale64bit force-pushed feature/store-basic-sanity-test from 6cceca4239 to 98744aa5e1

2023-08-31 14:23:08 +00:00

Compare

ale64bit requested review from storage-core-committers 2023-08-31 14:24:06 +00:00

ale64bit requested review from storage-core-developers 2023-08-31 14:24:15 +00:00

dstepanov-yadro approved these changes 2023-08-31 15:31:43 +00:00

fyrchik reviewed 2023-08-31 15:38:12 +00:00

pkg/local_object_storage/internal/storagetest/sanity.go

					
				@ -0,0 +15,4 @@

				// 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 {

fyrchik commented

2023-08-31 15:38:12 +00:00

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`?

ale64bit commented

2023-09-01 07:44:03 +00:00

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.

fyrchik commented

2023-09-01 08:03:13 +00:00

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?

ale64bit commented

2023-09-01 08:07:51 +00:00

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.

fyrchik commented

2023-09-01 08:28:08 +00:00

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.