storage: add range keys and mvccstats to gc bench #87261
Conversation
|
This change is |
aliher1911
force-pushed
the
add_range_tombstone_to_mvcc_gc_bench
branch
from
208f9ac
to
00f4bdd
Compare
aliher1911
force-pushed
the
add_range_tombstone_to_mvcc_gc_bench
branch
from
00f4bdd
to
7f91582
Compare
| for _, rangeTombstones := range numRangeTombstones { | ||
| b.Run(fmt.Sprintf("numRangeTs=%d", rangeTombstones), func(b *testing.B) { | ||
| for _, stats := range updateStats { | ||
| b.Run(fmt.Sprintf("updateStats=%t", stats), func(b *testing.B) { |
There was a problem hiding this comment.
Does the false variant here have any real value, since we'll always enable stats during real GC?
There was a problem hiding this comment.
It is helpful when comparing to previous releases mostly. Maybe we should remove unhelpful benchmarks later.
Maybe we should backport benchmarks like that to make life easier?
There was a problem hiding this comment.
No strong opinion either way. It's fine to keep it in as well.
| @@ -63,6 +63,8 @@ func BenchmarkMVCCGarbageCollect(b *testing.B) { | |||
| {2, []int{1}}, | |||
| {1024, []int{1, 16, 32, 512, 1015, 1023}}, | |||
| } | |||
| numRangeTombstones := []int{0, 1} | |||
There was a problem hiding this comment.
Would be nice to have a variant with 100 random range tombstones too, like the other benchmarks, but this is fine for now.
There was a problem hiding this comment.
That requires reverting key by key to ts by ts insertion. I did this change when experimenting with batch performance, but I'd rather add it separately.
Previously gc benchmarking code didn't include neither range key tombstones, nor mvcc stats calculations. This is not realistic as production always uses stats and it is a non-negligible part of the operation time. This commit adds option to enable stats calculation during benchmarking as well as adding underlying range key tombstone to include everything. Release justification: increasing bench coverage, non production. Release note: None
aliher1911
force-pushed
the
add_range_tombstone_to_mvcc_gc_bench
branch
from
7f91582
to
ab4b121
Compare
|
TFTR bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
Previously gc benchmarking code didn't include neither range key
tombstones, nor mvcc stats calculations.
This is not realistic as production always uses stats and it is
a non-negligible part of the operation time.
This commit adds option to enable stats calculation during
benchmarking as well as adding underlying range key tombstone
to include everything.
Release justification: increasing bench coverage, non production.
Release note: None