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.

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

batcheval: add range tombstone support for DeleteRange #77762

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -282,4 +282,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using
trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector string the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
version version 22.1-12 set the active cluster version in the format '<major>.<minor>'
version version 22.1-14 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,6 @@
<tr><td><code>trace.opentelemetry.collector</code></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.</td></tr>
<tr><td><code>trace.span_registry.enabled</code></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://<ui>/#/debug/tracez</td></tr>
<tr><td><code>trace.zipkin.collector</code></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>22.1-12</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
<tr><td><code>version</code></td><td>version</td><td><code>22.1-14</code></td><td>set the active cluster version in the format '<major>.<minor>'</td></tr>
</tbody>
</table>
8 changes: 6 additions & 2 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,13 @@ const (
// version is guaranteed to reside in a cluster where all nodes support range
// keys at the Pebble layer.
EnablePebbleFormatVersionRangeKeys

// TrigramInvertedIndexes enables the creation of trigram inverted indexes
// on strings.
TrigramInvertedIndexes

// RemoveGrantPrivilege is the last step to migrate from the GRANT privilege to WITH GRANT OPTION.
RemoveGrantPrivilege
// MVCCRangeTombstones enables the use of MVCC range tombstones.
MVCCRangeTombstones

// *************************************************
// Step (1): Add new versions here.
Expand Down Expand Up @@ -653,6 +653,10 @@ var versionsSingleton = keyedVersions{
Key: RemoveGrantPrivilege,
Version: roachpb.Version{Major: 22, Minor: 1, Internal: 12},
},
{
Key: MVCCRangeTombstones,
Version: roachpb.Version{Major: 22, Minor: 1, Internal: 14},
},

// *************************************************
// Step (2): Add new versions here.
Expand Down
5 changes: 3 additions & 2 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions pkg/kv/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,34 @@ func (b *Batch) DelRange(s, e interface{}, returnKeys bool) {
b.initResult(1, 0, notRaw, nil)
}

// ExperimentalDelRangeUsingTombstone deletes the rows between begin (inclusive)
// and end (exclusive) using an MVCC range tombstone. Callers must check the
// MVCCRangeTombstones version gate before using this.
//
// This method is EXPERIMENTAL: range tombstones are under active development,
// and have severe limitations including being ignored by all KV and MVCC APIs
// and only being stored in memory.
func (b *Batch) ExperimentalDelRangeUsingTombstone(s, e interface{}) {
start, err := marshalKey(s)
if err != nil {
b.initResult(0, 0, notRaw, err)
return
}
end, err := marshalKey(e)
if err != nil {
b.initResult(0, 0, notRaw, err)
return
}
b.appendReqs(&roachpb.DeleteRangeRequest{
RequestHeader: roachpb.RequestHeader{
Key: start,
EndKey: end,
},
UseExperimentalRangeTombstone: true,
})
b.initResult(1, 0, notRaw, nil)
}

// adminMerge is only exported on DB. It is here for symmetry with the
// other operations.
func (b *Batch) adminMerge(key interface{}) {
Expand Down
16 changes: 16 additions & 0 deletions pkg/kv/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,22 @@ func (db *DB) DelRange(
return r.Keys, err
}

// ExperimentalDelRangeUsingTombstone deletes the rows between begin (inclusive)
// and end (exclusive) using an MVCC range tombstone. Callers must check the
// MVCCRangeTombstones version gate before using this.
//
// This method is EXPERIMENTAL: range tombstones are under active development,
// and have severe limitations including being ignored by all KV and MVCC APIs
// and only being stored in memory.
func (db *DB) ExperimentalDelRangeUsingTombstone(
ctx context.Context, begin, end interface{},
) error {
b := &Batch{}
b.ExperimentalDelRangeUsingTombstone(begin, end)
_, err := getOneResult(db.Run(ctx, b), b)
return err
}

// AdminMerge merges the range containing key and the subsequent range. After
// the merge operation is complete, the range containing key will contain all of
// the key/value pairs of the subsequent range and the subsequent range will no
Expand Down
1 change: 1 addition & 0 deletions pkg/kv/kvserver/batcheval/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ go_test(
srcs = [
"cmd_add_sstable_test.go",
"cmd_clear_range_test.go",
"cmd_delete_range_test.go",
"cmd_end_transaction_test.go",
"cmd_export_test.go",
"cmd_get_test.go",
Expand Down
19 changes: 19 additions & 0 deletions pkg/kv/kvserver/batcheval/cmd_delete_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/errors"
)

func init() {
Expand Down Expand Up @@ -49,6 +50,24 @@ func DeleteRange(
h := cArgs.Header
reply := resp.(*roachpb.DeleteRangeResponse)

// Use experimental MVCC range tombstone if requested.
if args.UseExperimentalRangeTombstone {
if cArgs.Header.Txn != nil {
return result.Result{}, ErrTransactionUnsupported
}
if args.Inline {
return result.Result{}, errors.AssertionFailedf("Inline can't be used with range tombstones")
}
if args.ReturnKeys {
return result.Result{}, errors.AssertionFailedf(
"ReturnKeys can't be used with range tombstones")
}
maxIntents := storage.MaxIntentsPerWriteIntentError.Get(&cArgs.EvalCtx.ClusterSettings().SV)
err := storage.ExperimentalMVCCDeleteRangeUsingTombstone(
ctx, readWriter, cArgs.Stats, args.Key, args.EndKey, h.Timestamp, cArgs.Now, maxIntents)
return result.Result{}, err
}

var timestamp hlc.Timestamp
if !args.Inline {
timestamp = h.Timestamp
Expand Down
221 changes: 221 additions & 0 deletions pkg/kv/kvserver/batcheval/cmd_delete_range_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package batcheval_test

import (
"context"
"testing"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/require"
)

// TestDeleteRangeTombstone tests DeleteRange range tombstones directly, using
// only a Pebble engine.
//
// MVCC range tombstone logic is tested exhaustively in the MVCC history tests,
// this just tests the RPC plumbing.
func TestDeleteRangeTombstone(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

// Initial data for each test. x is point tombstone, [] is intent,
// o---o is range tombstone.
//
// 5 [i5]
// 4 c4
// 3 x
// 2 b2 d2 o-------o
// 1
// a b c d e f g h i
writeInitialData := func(t *testing.T, ctx context.Context, rw storage.ReadWriter) {
erikgrinaker marked this conversation as resolved.
Show resolved Hide resolved
t.Helper()
var localTS hlc.ClockTimestamp
txn := roachpb.MakeTransaction("test", nil /* baseKey */, roachpb.NormalUserPriority, hlc.Timestamp{WallTime: 5}, 0, 0)
require.NoError(t, storage.MVCCPut(ctx, rw, nil, roachpb.Key("b"), hlc.Timestamp{WallTime: 2}, localTS, roachpb.MakeValueFromString("b2"), nil))
require.NoError(t, storage.MVCCPut(ctx, rw, nil, roachpb.Key("c"), hlc.Timestamp{WallTime: 4}, localTS, roachpb.MakeValueFromString("c4"), nil))
require.NoError(t, storage.MVCCPut(ctx, rw, nil, roachpb.Key("d"), hlc.Timestamp{WallTime: 2}, localTS, roachpb.MakeValueFromString("d2"), nil))
require.NoError(t, storage.MVCCDelete(ctx, rw, nil, roachpb.Key("d"), hlc.Timestamp{WallTime: 3}, localTS, nil))
require.NoError(t, storage.MVCCPut(ctx, rw, nil, roachpb.Key("i"), hlc.Timestamp{WallTime: 5}, localTS, roachpb.MakeValueFromString("i5"), &txn))
require.NoError(t, storage.ExperimentalMVCCDeleteRangeUsingTombstone(ctx, rw, nil, roachpb.Key("f"), roachpb.Key("h"), hlc.Timestamp{WallTime: 3}, localTS, 0))
}

now := hlc.ClockTimestamp{Logical: 9}

testcases := map[string]struct {
start string
end string
ts int64
txn bool
inline bool
returnKeys bool
expectErr interface{} // error type, substring, or true (any)
}{
"above points succeed": {
start: "a",
end: "f",
ts: 10,
expectErr: nil,
},
"above range tombstone succeed": {
start: "f",
end: "h",
ts: 10,
expectErr: nil,
},
"transaction errors": {
start: "a",
end: "f",
ts: 10,
txn: true,
expectErr: batcheval.ErrTransactionUnsupported,
},
"inline errors": {
start: "a",
end: "f",
ts: 10,
inline: true,
expectErr: "Inline can't be used with range tombstones",
},
"returnKeys errors": {
start: "a",
end: "f",
ts: 10,
returnKeys: true,
expectErr: "ReturnKeys can't be used with range tombstones",
},
"intent errors with WriteIntentError": {
erikgrinaker marked this conversation as resolved.
Show resolved Hide resolved
start: "i",
end: "j",
ts: 10,
expectErr: &roachpb.WriteIntentError{},
},
"below point errors with WriteTooOldError": {
start: "a",
end: "d",
ts: 1,
expectErr: &roachpb.WriteTooOldError{},
},
"below range tombstone errors with WriteTooOldError": {
start: "f",
end: "h",
ts: 1,
expectErr: &roachpb.WriteTooOldError{},
},
}
for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
ctx := context.Background()
st := cluster.MakeTestingClusterSettings()
engine := storage.NewDefaultInMemForTesting()
defer engine.Close()

writeInitialData(t, ctx, engine)

rangeKey := storage.MVCCRangeKey{
StartKey: roachpb.Key(tc.start),
EndKey: roachpb.Key(tc.end),
Timestamp: hlc.Timestamp{WallTime: tc.ts},
}

var txn *roachpb.Transaction
if tc.txn {
tx := roachpb.MakeTransaction("txn", nil /* baseKey */, roachpb.NormalUserPriority, rangeKey.Timestamp, 0, 0)
txn = &tx
}

// Run the request.
var ms enginepb.MVCCStats
resp := &roachpb.DeleteRangeResponse{}
_, err := batcheval.DeleteRange(ctx, engine, batcheval.CommandArgs{
EvalCtx: (&batcheval.MockEvalCtx{ClusterSettings: st}).EvalContext(),
Stats: &ms,
Now: now,
Header: roachpb.Header{
Timestamp: rangeKey.Timestamp,
Txn: txn,
},
Args: &roachpb.DeleteRangeRequest{
RequestHeader: roachpb.RequestHeader{
Key: rangeKey.StartKey,
EndKey: rangeKey.EndKey,
},
UseExperimentalRangeTombstone: true,
Inline: tc.inline,
ReturnKeys: tc.returnKeys,
},
}, resp)

// Check the error.
if tc.expectErr != nil {
require.Error(t, err)
if b, ok := tc.expectErr.(bool); ok && b {
// any error is fine
} else if expectMsg, ok := tc.expectErr.(string); ok {
require.Contains(t, err.Error(), expectMsg)
} else if e, ok := tc.expectErr.(error); ok {
require.True(t, errors.HasType(err, e), "expected %T, got %v", e, err)
} else {
require.Fail(t, "invalid expectErr", "expectErr=%v", tc.expectErr)
}
return
}
require.NoError(t, err)

// Check that the range tombstone was written successfully.
iter := engine.NewMVCCIterator(storage.MVCCKeyAndIntentsIterKind, storage.IterOptions{
KeyTypes: storage.IterKeyTypeRangesOnly,
LowerBound: rangeKey.StartKey,
UpperBound: rangeKey.EndKey,
})
defer iter.Close()
iter.SeekGE(storage.MVCCKey{Key: rangeKey.StartKey})

var seen storage.MVCCRangeKeyValue
for {
ok, err := iter.Valid()
require.NoError(t, err)
if !ok {
break
}
require.True(t, ok)
for _, rkv := range iter.RangeKeys() {
if rkv.RangeKey.Timestamp.Equal(rangeKey.Timestamp) {
if len(seen.RangeKey.StartKey) == 0 {
seen = rkv.Clone()
} else {
seen.RangeKey.EndKey = rkv.RangeKey.EndKey.Clone()
require.Equal(t, seen.Value, rkv.Value)
}
break
}
}
iter.Next()
}
require.Equal(t, rangeKey, seen.RangeKey)

value, err := storage.DecodeMVCCValue(seen.Value)
require.NoError(t, err)
require.True(t, value.IsTombstone())
require.Equal(t, now, value.LocalTimestamp)

// TODO(erikgrinaker): This should test MVCC stats when implemented.
})
}
}
Loading