[#81] node: Add basic read/write benchmarks for substorages #84
No reviewers
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#84
Loading…
Reference in a new issue
No description provided.
Delete branch "ale64bit/frostfs-node:fix/81-substorage_benchmark"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Signed-off-by: Alejandro Lopez a.lopez@yadro.com
Close #81
@ -0,0 +1,168 @@
// memstore implements a memory-backed common.Storage for testing purposes.
Package comment should start with a
Package
https://tip.golang.org/doc/commentdone
@ -0,0 +190,4 @@
func addressFromObject(obj *objectSDK.Object) oid.Address {
var addr oid.Address
if id, isSet := obj.ID(); isSet {
Both object ID and container ID MUST always be set, we can ignore the second return value or panic here.
done. Can we force it to be constructed properly then? (maybe hide it under an interface and have a
New
function that ensures those things are actually set or return an error).The problem is that object starts being constructed before we set an actual ID, so not having ID is a possibility from the POV of code.
@ -0,0 +258,4 @@
}
// seqObjGenerator is an objectGenerator that generates entries with random payloads of size objSize and sequential IDs.
type seqObjGenerator struct {
I doubt this has any value: object ID is a hash, so
sequential
load is not something we could see in the real life.However, we could write a benchmark (somewhat sequential) for putting objects in the same container vs putting to multiple containers.
True, but having the sequential benchmark can provide a hint to whether the substorage should do something about the sequentiality of the keys in case sequential writes happen to be beneficial for a particular implementation. I think it's a good comparison point in either case.
My point is that there is no sequential load in our system, so we do not need to optimize for this case. Because of this the value of the benchmark itself is limited: another storage can have it's different view on what is sequential, for example.
My suggestion is to use different OID but the same CID as an approximation of possible "sequential" load -- the load is real and storage component can choose to store objects by CID_OID key to make PUTs to the same container "more sequential".
@ -0,0 +331,4 @@
}
// Generates an object with random payload and the given address and size.
// TODO(ale64bit): there's some testing-related dupes in many places. Probably worth
Let's accompany all TODOs with a link to some issue?
done
30ec5fedeb
to4aecd3f79a
4aecd3f79a
to1b58f60772
@ -39,6 +39,8 @@ require (
gopkg.in/yaml.v3 v3.0.1
)
require golang.org/x/exp v0.0.0-20230224173230-c95f2b4c22f2
Was it addded manually? I am wondering why it wasn't included in a previous
require
group.nope. I guess from
go mod tidy
. Maybe it was only indirect before.@ -0,0 +46,4 @@
// Decompress the data.
var err error
if data, err = s.compression.Decompress(data); err != nil {
return common.GetRes{}, logicerr.Wrap(fmt.Errorf("could not decompress object data: %w", err))
is that a logic error?
I guess they aren't. Fixed.
@ -0,0 +76,4 @@
}
return common.GetRangeRes{
Data: payload[from:to],
can we have some problems here because of
mu
not being held?I'm not sure what kind of problem you have in mind. Can you elaborate? (
mu
is held duringGet
).@ -0,0 +121,4 @@
if _, exists := s.objs[key]; exists {
delete(s.objs, key)
return common.DeleteRes{}, nil
} else {
redundant
else
?done
1b58f60772
toc7d231f064
@ -0,0 +274,4 @@
return addr
}
func TestSeqAddrGenerator(t *testing.T) {
Should we remove this now?
I removed the sequential test for write benchmark, but the generator is still used (it's useful for example, when you want to fill an instance and execute a workload where you want to control the ratio of not_found to found, or know exactly which keys are present more generally, without storing them somewhere).