Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: don't use ClearRange point deletes with estimated MVCC stats #74674

Merged
merged 1 commit into from
Jan 13, 2022
Merged

kvserver: don't use ClearRange point deletes with estimated MVCC stats #74674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions pkg/kv/kvserver/batcheval/cmd_clear_range.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,15 @@ func ClearRange(
// If the total size of data to be cleared is less than
// clearRangeBytesThreshold, clear the individual values with an iterator,
// instead of using a range tombstone (inefficient for small ranges).
if total := statsDelta.Total(); total < ClearRangeBytesThreshold {
log.VEventf(ctx, 2, "delta=%d < threshold=%d; using non-range clear", total, ClearRangeBytesThreshold)
//
// However, don't do this if the stats contain estimates -- this can only
// happen when we're clearing an entire range and we're using the existing
// range stats. We've seen cases where these estimates are wildly inaccurate
// (even negative), and it's better to drop an unnecessary range tombstone
// than to submit a huge write batch that'll get rejected by Raft.
if statsDelta.ContainsEstimates == 0 && statsDelta.Total() < ClearRangeBytesThreshold {
log.VEventf(ctx, 2, "delta=%d < threshold=%d; using non-range clear",
statsDelta.Total(), ClearRangeBytesThreshold)
iter := readWriter.NewMVCCIterator(storage.MVCCKeyAndIntentsIterKind, storage.IterOptions{
LowerBound: from,
UpperBound: to,
Expand Down Expand Up @@ -154,7 +161,7 @@ func computeStatsDelta(
}
// If we took the fast path but race is enabled, assert stats were correctly computed.
if fast {
delta.ContainsEstimates = computed.ContainsEstimates
computed.ContainsEstimates = delta.ContainsEstimates // retained for tests under race
if !delta.Equal(computed) {
log.Fatalf(ctx, "fast-path MVCCStats computation gave wrong result: diff(fast, computed) = %s",
pretty.Diff(delta, computed))
Expand Down
19 changes: 16 additions & 3 deletions pkg/kv/kvserver/batcheval/cmd_clear_range_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) {
overFull := ClearRangeBytesThreshold/len(valueStr) + 1
tests := []struct {
keyCount int
estimatedStats bool
expClearIterCount int
expClearRangeCount int
}{
Expand All @@ -85,6 +86,13 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) {
expClearIterCount: 0,
expClearRangeCount: 1,
},
// Estimated stats always use ClearRange.
{
keyCount: 1,
estimatedStats: true,
expClearIterCount: 0,
expClearRangeCount: 1,
},
}

for _, test := range tests {
Expand All @@ -100,6 +108,9 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) {
t.Fatal(err)
}
}
if test.estimatedStats {
stats.ContainsEstimates++
}

batch := &wrappedBatch{Batch: eng.NewBatch()}
defer batch.Close()
Expand All @@ -126,10 +137,12 @@ func TestCmdClearRangeBytesThreshold(t *testing.T) {
t.Fatal(err)
}

// Verify cArgs.Stats is equal to the stats we wrote.
// Verify cArgs.Stats is equal to the stats we wrote, ignoring some values.
newStats := stats
newStats.SysBytes, newStats.SysCount, newStats.AbortSpanBytes = 0, 0, 0 // ignore these values
cArgs.Stats.SysBytes, cArgs.Stats.SysCount, cArgs.Stats.AbortSpanBytes = 0, 0, 0 // these too, as GC threshold is updated
newStats.ContainsEstimates, cArgs.Stats.ContainsEstimates = 0, 0
newStats.SysBytes, cArgs.Stats.SysBytes = 0, 0
newStats.SysCount, cArgs.Stats.SysCount = 0, 0
newStats.AbortSpanBytes, cArgs.Stats.AbortSpanBytes = 0, 0
newStats.Add(*cArgs.Stats)
newStats.AgeTo(0) // pin at LastUpdateNanos==0
if !newStats.Equal(enginepb.MVCCStats{}) {
Expand Down