Skip to content

Commit

Permalink
Merge cockroachdb#86129
Browse files Browse the repository at this point in the history 
86129: rangefeed: bench mvcc range tombstones in BenchmarkCatchUpScan r=erikgrinaker a=tbg

```
go test ./pkg/kv/kvserver/rangefeed/ -run - -count 10 -bench BenchmarkCatchUpScan 2>&1 | tee bench.txt
for flavor in numRangeKeys=0 numRangeKeys=1 numRangeKeys=100; do grep -E "${flavor}[^0-9]+" bench.txt | sed -E "s/${flavor}+/X/" > $flavor.txt; done && benchstat numRangeKeys\=*
```

```
benchstat numRangeKeys\={0,1}.txt

name                                                    old time/op  new time/op   delta
CatchUpScan/random-keys/withDiff=true/perc=0.00/X-24     663ms ± 1%    870ms ± 1%  +31.18%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=50.00/X-24    1.20s ± 1%    2.05s ± 0%  +71.31%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=75.00/X-24    1.19s ± 1%    1.62s ± 1%  +36.46%  (p=0.000 n=10+9)
CatchUpScan/random-keys/withDiff=true/perc=95.00/X-24    704ms ± 4%    882ms ± 1%  +25.31%  (p=0.000 n=10+9)
CatchUpScan/random-keys/withDiff=true/perc=99.00/X-24    580ms ± 1%    733ms ± 1%  +26.30%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=0.00/X-24    993ms ± 1%   1215ms ± 2%  +22.33%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=50.00/X-24   821ms ± 1%   1044ms ± 1%  +27.11%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=75.00/X-24   694ms ± 1%    843ms ± 1%  +21.46%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=95.00/X-24   573ms ± 1%    706ms ± 1%  +23.19%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=99.00/X-24   554ms ± 0%    682ms ± 1%  +23.10%  (p=0.000 n=8+10)
CatchUpScan/mixed-case/withDiff=true/perc=0.00/X-24      670ms ± 1%    871ms ± 1%  +29.92%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=true/perc=50.00/X-24     1.33s ± 0%    2.31s ± 0%  +73.67%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=true/perc=75.00/X-24     1.14s ± 1%    1.93s ± 1%  +69.53%  (p=0.000 n=9+10)
CatchUpScan/mixed-case/withDiff=true/perc=95.00/X-24     233ms ± 1%    289ms ± 1%  +24.20%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=true/perc=99.00/X-24     195ms ± 1%    235ms ± 1%  +20.49%  (p=0.000 n=8+10)
CatchUpScan/mixed-case/withDiff=false/perc=0.00/X-24     1.01s ± 1%    1.22s ± 1%  +21.81%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=50.00/X-24    839ms ± 1%   1052ms ± 1%  +25.33%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=75.00/X-24    734ms ± 0%    948ms ± 1%  +29.19%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=95.00/X-24    214ms ± 3%    245ms ± 1%  +14.12%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=99.00/X-24    190ms ± 1%    222ms ± 1%  +16.81%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=0.00/X-24     643ms ± 2%    845ms ± 1%  +31.30%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=50.00/X-24    313ms ± 2%    422ms ± 2%  +35.11%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=75.00/X-24    152ms ± 2%    205ms ± 1%  +34.65%  (p=0.000 n=10+8)
CatchUpScan/linear-keys/withDiff=true/perc=95.00/X-24   29.3ms ± 5%   40.0ms ± 1%  +36.49%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=99.00/X-24   5.78ms ± 2%   8.05ms ± 3%  +39.24%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=0.00/X-24    940ms ± 1%   1173ms ± 1%  +24.80%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=50.00/X-24   464ms ± 1%    539ms ± 1%  +16.20%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=75.00/X-24   225ms ± 2%    261ms ± 2%  +16.32%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=95.00/X-24  42.9ms ± 1%   50.4ms ± 2%  +17.58%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=99.00/X-24  8.44ms ± 1%  10.12ms ± 2%  +19.91%  (p=0.000 n=8+10)
```

```
benchstat numRangeKeys\={0,100}.txt

name                                                    old time/op  new time/op  delta
CatchUpScan/random-keys/withDiff=true/perc=0.00/X-24     663ms ± 1%   945ms ± 1%   +42.51%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=50.00/X-24    1.20s ± 1%   2.58s ± 0%  +115.84%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=75.00/X-24    1.19s ± 1%   1.62s ± 1%   +35.94%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=95.00/X-24    704ms ± 4%  1022ms ± 1%   +45.12%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=true/perc=99.00/X-24    580ms ± 1%   859ms ± 0%   +48.05%  (p=0.000 n=10+9)
CatchUpScan/random-keys/withDiff=false/perc=0.00/X-24    993ms ± 1%  1228ms ± 1%   +23.61%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=50.00/X-24   821ms ± 1%  1057ms ± 1%   +28.73%  (p=0.000 n=10+9)
CatchUpScan/random-keys/withDiff=false/perc=75.00/X-24   694ms ± 1%   923ms ± 1%   +33.04%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=95.00/X-24   573ms ± 1%   823ms ± 1%   +43.56%  (p=0.000 n=10+10)
CatchUpScan/random-keys/withDiff=false/perc=99.00/X-24   554ms ± 0%   801ms ± 1%   +44.51%  (p=0.000 n=8+10)
CatchUpScan/mixed-case/withDiff=true/perc=0.00/X-24      670ms ± 1%   956ms ± 1%   +42.65%  (p=0.000 n=10+9)
CatchUpScan/mixed-case/withDiff=true/perc=50.00/X-24     1.33s ± 0%   3.15s ± 0%  +136.94%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=true/perc=75.00/X-24     1.14s ± 1%   2.56s ± 0%  +124.96%  (p=0.000 n=9+10)
CatchUpScan/mixed-case/withDiff=true/perc=95.00/X-24     233ms ± 1%   395ms ± 0%   +69.80%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=true/perc=99.00/X-24     195ms ± 1%   347ms ± 1%   +78.15%  (p=0.000 n=8+9)
CatchUpScan/mixed-case/withDiff=false/perc=0.00/X-24     1.01s ± 1%   1.24s ± 1%   +23.17%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=50.00/X-24    839ms ± 1%  1066ms ± 1%   +27.10%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=75.00/X-24    734ms ± 0%   964ms ± 1%   +31.29%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=95.00/X-24    214ms ± 3%   353ms ± 1%   +64.88%  (p=0.000 n=10+10)
CatchUpScan/mixed-case/withDiff=false/perc=99.00/X-24    190ms ± 1%   329ms ± 1%   +72.95%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=0.00/X-24     643ms ± 2%   916ms ± 2%   +42.40%  (p=0.000 n=10+9)
CatchUpScan/linear-keys/withDiff=true/perc=50.00/X-24    313ms ± 2%   466ms ± 1%   +49.05%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=75.00/X-24    152ms ± 2%   209ms ± 1%   +37.74%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=true/perc=95.00/X-24   29.3ms ± 5%  40.3ms ± 1%   +37.42%  (p=0.000 n=10+9)
CatchUpScan/linear-keys/withDiff=true/perc=99.00/X-24   5.78ms ± 2%  7.70ms ± 1%   +33.23%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=0.00/X-24    940ms ± 1%  1189ms ± 2%   +26.60%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=50.00/X-24   464ms ± 1%   541ms ± 1%   +16.67%  (p=0.000 n=10+9)
CatchUpScan/linear-keys/withDiff=false/perc=75.00/X-24   225ms ± 2%   262ms ± 1%   +16.78%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=95.00/X-24  42.9ms ± 1%  50.3ms ± 1%   +17.21%  (p=0.000 n=10+10)
CatchUpScan/linear-keys/withDiff=false/perc=99.00/X-24  8.44ms ± 1%  9.89ms ± 1%   +17.26%  (p=0.000 n=8+10)
```


Fixes cockroachdb#86128.

Release note: None
Release justification: critical bug fix & benchmarking for new functionality (mvcc range deletions)


Co-authored-by: Tobias Grieger <[email protected]>
  • Loading branch information
@tbg
craig[bot] and tbg committed Aug 16, 2022
2 parents 8f1b11e + 7e0583c commit 5d7e232
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 10 deletions.
14 changes: 12 additions & 2 deletions pkg/kv/kvserver/rangefeed/catchup_scan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,34 @@ func (i *CatchUpIterator) CatchUpScan(outputFn outputEventFn, withDiff bool) err

// Emit any new MVCC range tombstones when their start key is encountered.
// Range keys can currently only be MVCC range tombstones.
// We need to verify that the range tombstone is visible at the catch-up
// timestamp, since we might have come here after a call to NextIgnoringTime.
//
// TODO(erikgrinaker): Find a faster/better way to detect range key changes
// that doesn't involve constant comparisons. Pebble probably already knows,
// we just need a way to ask it.
// Note that byte slice comparison in Go is smart enough to immediately bail
// if lengths are different. However, it isn't smart enough to compare from
// the end, which would really help since our keys share prefixes.
if hasRange {
if rangeBounds := i.RangeBounds(); !rangeBounds.Key.Equal(rangeKeysStart) {
rangeKeysStart = append(rangeKeysStart[:0], rangeBounds.Key...)

// Emit events for these MVCC range tombstones, in chronological order.
versions := i.RangeKeys().Versions
for i := len(versions) - 1; i >= 0; i-- {
for j := len(versions) - 1; j >= 0; j-- {
if !i.startTime.LessEq(versions[j].Timestamp) {
// This range tombstone isn't visible by this catch-up scan.
continue
}

var span roachpb.Span
a, span.Key = a.Copy(rangeBounds.Key, 0)
a, span.EndKey = a.Copy(rangeBounds.EndKey, 0)
err := outputFn(&roachpb.RangeFeedEvent{
DeleteRange: &roachpb.RangeFeedDeleteRange{
Span: span,
Timestamp: versions[i].Timestamp,
Timestamp: versions[j].Timestamp,
},
})
if err != nil {
Expand Down
64 changes: 56 additions & 8 deletions pkg/kv/kvserver/rangefeed/catchup_scan_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ import (
"github.com/cockroachdb/errors/oserror"
"github.com/cockroachdb/pebble"
"github.com/cockroachdb/pebble/vfs"
"github.com/stretchr/testify/require"
)

func runCatchUpBenchmark(b *testing.B, emk engineMaker, opts benchOptions) {
func runCatchUpBenchmark(b *testing.B, emk engineMaker, opts benchOptions) (numEvents int) {
eng, _ := setupData(context.Background(), b, emk, opts.dataOpts)
defer eng.Close()
startKey := roachpb.Key(encoding.EncodeUvarintAscending([]byte("key-"), uint64(0)))
Expand All @@ -59,8 +60,17 @@ func runCatchUpBenchmark(b *testing.B, emk engineMaker, opts benchOptions) {
if counter < 1 {
b.Fatalf("didn't emit any events!")
}
if numEvents == 0 {
// Preserve number of events so that caller can compare it between
// different invocations that it knows should not affect number of
// events.
numEvents = counter
}
// Number of events can't change between iterations.
require.Equal(b, numEvents, counter)
}()
}
return numEvents
}

func BenchmarkCatchUpScan(b *testing.B) {
Expand Down Expand Up @@ -136,11 +146,24 @@ func BenchmarkCatchUpScan(b *testing.B) {
wallTime := int64((5 * (float64(numKeys)*tsExcludePercent + 1)))
ts := hlc.Timestamp{WallTime: wallTime}
b.Run(fmt.Sprintf("perc=%2.2f", tsExcludePercent*100), func(b *testing.B) {
runCatchUpBenchmark(b, setupMVCCPebble, benchOptions{
dataOpts: do,
ts: ts,
withDiff: withDiff,
})
for _, numRangeKeys := range []int{0, 1, 100} {
b.Run(fmt.Sprintf("numRangeKeys=%d", numRangeKeys), func(b *testing.B) {
do := do
do.numRangeKeys = numRangeKeys
n := runCatchUpBenchmark(b, setupMVCCPebble, benchOptions{
dataOpts: do,
ts: ts,
withDiff: withDiff,
})
// We shouldn't be seeing the range deletions returned in this
// benchmark since they are at timestamp 1 and we catch up at
// a timestamp >= 5 (which corresponds to tsExcludePercent ==
// 0). Note that the oldest key is always excluded, since the
// floor for wallTime is 5 and that's the oldest key's
// timestamp but the start timestamp is exclusive.
require.EqualValues(b, int64(numKeys)-wallTime/5, n)
})
}
})
}
})
Expand All @@ -155,6 +178,7 @@ type benchDataOptions struct {
randomKeyOrder bool
readOnlyEngine bool
lBaseMaxBytes int64
numRangeKeys int
}

type benchOptions struct {
Expand Down Expand Up @@ -212,8 +236,8 @@ func setupData(
if opts.readOnlyEngine {
readOnlyStr = "_readonly"
}
loc := fmt.Sprintf("rangefeed_bench_data_%s%s_%d_%d_%d",
orderStr, readOnlyStr, opts.numKeys, opts.valueBytes, opts.lBaseMaxBytes)
loc := fmt.Sprintf("rangefeed_bench_data_%s%s_%d_%d_%d_%d",
orderStr, readOnlyStr, opts.numKeys, opts.valueBytes, opts.lBaseMaxBytes, opts.numRangeKeys)
exists := true
if _, err := os.Stat(loc); oserror.IsNotExist(err) {
exists = false
Expand Down Expand Up @@ -245,6 +269,30 @@ func setupData(
})
}

writeRangeKeys := func(b testing.TB, wallTime int) {
batch := eng.NewBatch()
defer batch.Close()
for i := 0; i < opts.numRangeKeys; i++ {
// NB: regular keys are written at ts 5+, so this is below any of the
// regular writes and thus won't delete anything.
ts := hlc.Timestamp{WallTime: int64(wallTime), Logical: int32(i + 1)}
start := rng.Intn(opts.numKeys)
end := start + rng.Intn(opts.numKeys-start) + 1
// As a special case, if we're only writing one range key, write it across
// the entire span.
if opts.numRangeKeys == 1 {
start = 0
end = opts.numKeys + 1
}
startKey := roachpb.Key(encoding.EncodeUvarintAscending([]byte("key-"), uint64(start)))
endKey := roachpb.Key(encoding.EncodeUvarintAscending([]byte("key-"), uint64(end)))
require.NoError(b, storage.MVCCDeleteRangeUsingTombstone(
ctx, batch, nil, startKey, endKey, ts, hlc.ClockTimestamp{}, nil, nil, false, 0, nil))
}
require.NoError(b, batch.Commit(false /* sync */))
}
writeRangeKeys(b, 1 /* wallTime */)

writeKey := func(batch storage.Batch, idx int, pos int) {
key := keys[idx]
value := roachpb.MakeValueFromBytes(randutil.RandBytes(rng, opts.valueBytes))
Expand Down
7 changes: 7 additions & 0 deletions pkg/kv/kvserver/rangefeed/catchup_scan_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ import (
// https://github.com/cockroachdb/cockroach/issues/82715
//
// For now, see rangefeed_external_test.go for rudimentary range key tests.
//
// To invoke and compare on the numRangeKeys dimension:
//
// go test ./pkg/kv/kvserver/rangefeed/ -run - -count 10 -bench BenchmarkCatchUpScan 2>&1 | tee bench.txt
// for flavor in numRangeKeys=0 numRangeKeys=1 numRangeKeys=100; do grep -E "${flavor}[^0-9]+" bench.txt | sed -E "s/${flavor}+/X/" > $flavor.txt; done
// benchstat numRangeKeys\={0,1}.txt
// benchstat numRangeKeys\={0,100}.txt
func TestCatchupScan(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down

0 comments on commit 5d7e232

Please sign in to comment.