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

storage: revert experimental MVCC range tombstones #76921

Merged
merged 3 commits into from
Feb 23, 2022
Merged

storage: revert experimental MVCC range tombstones #76921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3b79e9
Revert "batcheval: add MVCC-compliant `RevertRange` variant"
erikgrinaker Feb 23, 2022
f313d66
Revert "batcheval: add range tombstone support for `DeleteRange`"
erikgrinaker Feb 23, 2022
4b79046
Revert "storage: add experimental MVCC range tombstone primitives"
erikgrinaker Feb 23, 2022
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
9 changes: 2 additions & 7 deletions pkg/clusterversion/cockroach_versions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,6 @@ const (
// BackupResolutionInJob defaults to resolving backup destinations during the
// execution of a backup job rather than during planning.
BackupResolutionInJob
// ExperimentalMVCCRangeTombstones enables the use of highly experimental MVCC
// range tombstones.
ExperimentalMVCCRangeTombstones
// LooselyCoupledRaftLogTruncation allows the cluster to reduce the coupling
// for raft log truncation, by allowing each replica to treat a truncation
// proposal as an upper bound on what should be truncated.
Expand Down Expand Up @@ -494,14 +491,12 @@ var versionsSingleton = keyedVersions{
Key: EnablePebbleFormatVersionRangeKeys,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 74},
},

{
Key: BackupResolutionInJob,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 76},
},
{
Key: ExperimentalMVCCRangeTombstones,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 78},
},
// Internal: 78 was reverted (ExperimentalMVCCRangeTombstones)
{
Key: LooselyCoupledRaftLogTruncation,
Version: roachpb.Version{Major: 21, Minor: 2, Internal: 80},
Expand Down
7 changes: 3 additions & 4 deletions pkg/clusterversion/key_string.go
View file
Open in desktop

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

28 changes: 0 additions & 28 deletions pkg/kv/batch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,34 +649,6 @@ 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. The caller must check
// storage.CanUseExperimentalMVCCRangeTombstones() 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: 0 additions & 16 deletions pkg/kv/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,22 +546,6 @@ func (db *DB) DelRange(
return r.Keys, err
}

// ExperimentalDelRangeUsingTombstone deletes the rows between begin (inclusive)
// and end (exclusive) using an MVCC range tombstone. The caller must check
// storage.CanUseExperimentalMVCCRangeTombstones() 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
22 changes: 0 additions & 22 deletions pkg/kv/kvserver/batcheval/cmd_delete_range.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ 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 @@ -50,27 +49,6 @@ func DeleteRange(
h := cArgs.Header
reply := resp.(*roachpb.DeleteRangeResponse)

// Use experimental MVCC range tombstone if requested. The caller is expected
// to have checked storage.CanUseExperimentalMVCCRangeTombstones() first.
//
// TODO(erikgrinaker): Add integration tests for this.
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, maxIntents)
return result.Result{}, err
}

var timestamp hlc.Timestamp
if !args.Inline {
timestamp = h.Timestamp
Expand Down
44 changes: 15 additions & 29 deletions pkg/kv/kvserver/batcheval/cmd_revert_range.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,8 @@ func isEmptyKeyTimeRange(
// that there is *a* key in the SST that is in the time range. Thus we should
// proceed to iteration that actually checks timestamps on each key.
iter := readWriter.NewMVCCIterator(storage.MVCCKeyIterKind, storage.IterOptions{
// TODO(erikgrinaker): Make sure TBIs respect range keys too.
KeyTypes: storage.IterKeyTypePointsAndRanges, // revert any range keys as well
LowerBound: from,
UpperBound: to,
MinTimestampHint: since.Next(), // exclusive
MaxTimestampHint: until,
LowerBound: from, UpperBound: to,
MinTimestampHint: since.Next() /* make exclusive */, MaxTimestampHint: until,
})
defer iter.Close()
iter.SeekGE(storage.MVCCKey{Key: from})
Expand All @@ -82,39 +78,29 @@ func RevertRange(

args := cArgs.Args.(*roachpb.RevertRangeRequest)
reply := resp.(*roachpb.RevertRangeResponse)
pd := result.Result{
Replicated: kvserverpb.ReplicatedEvalResult{
MVCCHistoryMutation: &kvserverpb.ReplicatedEvalResult_MVCCHistoryMutation{
Spans: []roachpb.Span{{Key: args.Key, EndKey: args.EndKey}},
},
},
}

if empty, err := isEmptyKeyTimeRange(
readWriter, args.Key, args.EndKey, args.TargetTime, cArgs.Header.Timestamp,
); err != nil {
return result.Result{}, err
} else if empty {
log.VEventf(ctx, 2, "no keys to revert in specified time range")
log.VEventf(ctx, 2, "no keys to clear in specified time range")
return result.Result{}, nil
}

log.VEventf(ctx, 2, "reverting keys with timestamp (%v, %v]",
args.TargetTime, cArgs.Header.Timestamp)
log.VEventf(ctx, 2, "clearing keys with timestamp (%v, %v]", args.TargetTime, cArgs.Header.Timestamp)

var pd result.Result
var resume *roachpb.Span
var err error
if args.ExperimentalPreserveHistory {
const deleteRangeThreshold = 100
maxIntents := storage.MaxIntentsPerWriteIntentError.Get(&cArgs.EvalCtx.ClusterSettings().SV)
// TODO(erikgrinaker): Write a test for this once MVCC range tombstones are
// properly written to batches and replicated.
// TODO(erikgrinaker): Test that this records MVCC logical ops correctly.
resume, err = storage.ExperimentalMVCCRevertRange(ctx, readWriter, cArgs.Stats,
args.Key, args.EndKey, cArgs.Header.Timestamp, args.TargetTime, deleteRangeThreshold,
cArgs.Header.MaxSpanRequestKeys, maxRevertRangeBatchBytes, maxIntents)
} else {
resume, err = storage.MVCCClearTimeRange(ctx, readWriter, cArgs.Stats, args.Key, args.EndKey,
args.TargetTime, cArgs.Header.Timestamp, cArgs.Header.MaxSpanRequestKeys,
maxRevertRangeBatchBytes, args.EnableTimeBoundIteratorOptimization)
pd.Replicated.MVCCHistoryMutation = &kvserverpb.ReplicatedEvalResult_MVCCHistoryMutation{
Spans: []roachpb.Span{{Key: args.Key, EndKey: args.EndKey}},
}
}
resume, err := storage.MVCCClearTimeRange(ctx, readWriter, cArgs.Stats, args.Key, args.EndKey,
args.TargetTime, cArgs.Header.Timestamp, cArgs.Header.MaxSpanRequestKeys,
maxRevertRangeBatchBytes,
args.EnableTimeBoundIteratorOptimization)
if err != nil {
return result.Result{}, err
}
Expand Down
15 changes: 0 additions & 15 deletions pkg/kv/kvserver/rangefeed/task_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,6 @@ func (s *testIterator) curKV() storage.MVCCKeyValue {
return s.kvs[s.cur]
}

// HasPointAndRange implements SimpleMVCCIterator.
func (s *testIterator) HasPointAndRange() (bool, bool) {
panic("not implemented")
}

// RangeBounds implements SimpleMVCCIterator.
func (s *testIterator) RangeBounds() (roachpb.Key, roachpb.Key) {
panic("not implemented")
}

// RangeTombstones implements SimpleMVCCIterator.
func (s *testIterator) RangeKeys() []storage.MVCCRangeKeyValue {
panic("not implemented")
}

func TestInitResolvedTSScan(t *testing.T) {
defer leaktest.AfterTest(t)()
startKey := roachpb.RKey("d")
Expand Down
31 changes: 0 additions & 31 deletions pkg/kv/kvserver/spanset/batch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,21 +176,6 @@ func (i *MVCCIterator) UnsafeValue() []byte {
return i.i.UnsafeValue()
}

// HasPointAndRange implements SimpleMVCCIterator.
func (i *MVCCIterator) HasPointAndRange() (bool, bool) {
return i.i.HasPointAndRange()
}

// RangeBounds implements SimpleMVCCIterator.
func (i *MVCCIterator) RangeBounds() (roachpb.Key, roachpb.Key) {
return i.i.RangeBounds()
}

// RangeKeys implements SimpleMVCCIterator.
func (i *MVCCIterator) RangeKeys() []storage.MVCCRangeKeyValue {
return i.i.RangeKeys()
}

// ComputeStats is part of the storage.MVCCIterator interface.
func (i *MVCCIterator) ComputeStats(
start, end roachpb.Key, nowNanos int64,
Expand Down Expand Up @@ -614,22 +599,6 @@ func (s spanSetWriter) ClearIterRange(iter storage.MVCCIterator, start, end roac
return s.w.ClearIterRange(iter, start, end)
}

func (s spanSetWriter) ExperimentalPutMVCCRangeKey(
rangeKey storage.MVCCRangeKey, value []byte,
) error {
if err := s.checkAllowedRange(rangeKey.StartKey, rangeKey.EndKey); err != nil {
return err
}
return s.w.ExperimentalPutMVCCRangeKey(rangeKey, value)
}

func (s spanSetWriter) ExperimentalClearMVCCRangeKey(rangeKey storage.MVCCRangeKey) error {
if err := s.checkAllowedRange(rangeKey.StartKey, rangeKey.EndKey); err != nil {
return err
}
return s.w.ExperimentalClearMVCCRangeKey(rangeKey)
}

func (s spanSetWriter) Merge(key storage.MVCCKey, value []byte) error {
if s.spansOnly {
if err := s.spans.CheckAllowed(SpanReadWrite, roachpb.Span{Key: key.Key}); err != nil {
Expand Down
19 changes: 5 additions & 14 deletions pkg/roachpb/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,8 +778,8 @@ func (crr *ClearRangeRequest) ShallowCopy() Request {
}

// ShallowCopy implements the Request interface.
func (rrr *RevertRangeRequest) ShallowCopy() Request {
shallowCopy := *rrr
func (crr *RevertRangeRequest) ShallowCopy() Request {
shallowCopy := *crr
return &shallowCopy
}

Expand Down Expand Up @@ -1233,10 +1233,6 @@ func (*DeleteRequest) flags() flag {
}

func (drr *DeleteRangeRequest) flags() flag {
// DeleteRangeRequest using MVCC range tombstones cannot be transactional.
if drr.UseExperimentalRangeTombstone {
return isWrite | isRange | isAlone | appliesTSCache
}
// DeleteRangeRequest has different properties if the "inline" flag is set.
// This flag indicates that the request is deleting inline MVCC values,
// which cannot be deleted transactionally - inline DeleteRange will thus
Expand Down Expand Up @@ -1270,14 +1266,9 @@ func (*ClearRangeRequest) flags() flag {
return isWrite | isRange | isAlone | bypassesReplicaCircuitBreaker
}

// Note that RevertRange commands cannot be part of a transaction, as they
// either clear MVCC versions or write MVCC range tombstones, neither of which
// is supported within transactions.
func (rrr *RevertRangeRequest) flags() flag {
if rrr.ExperimentalPreserveHistory {
return isRead | isWrite | isRange | isAlone | updatesTSCache | appliesTSCache |
bypassesReplicaCircuitBreaker
}
// Note that RevertRange commands cannot be part of a transaction as
// they clear all MVCC versions above their target time.
func (*RevertRangeRequest) flags() flag {
return isWrite | isRange | isAlone | bypassesReplicaCircuitBreaker
}

Expand Down
37 changes: 5 additions & 32 deletions pkg/roachpb/api.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,17 +348,6 @@ message DeleteRangeRequest {
// Inline values cannot be deleted transactionally; a DeleteRange with
// "inline" set to true will fail if it is executed within a transaction.
bool inline = 4;
// If enabled, the range is deleted using an MVCC range tombstone, which is a
// cheap constant-time operation. This option cannot be used in a transaction,
// and it cannot be combined with Inline or ReturnKeys.
//
// The caller must check storage.CanUseExperimentalMVCCRangeTombstones()
// before enabling this parameter.
//
// This parameter 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.
bool use_experimental_range_tombstone = 5;
}

// A DeleteRangeResponse is the return value from the DeleteRange()
Expand Down Expand Up @@ -402,34 +391,18 @@ message ClearRangeResponse {
}


// A RevertRangeRequest specifies a range of keys to revert to some past time.
// By default, it will clear all revision more recent that TargetTime from the
// underlying engine. However, this violates several guarantees including MVCC
// immutability, the closed timestamp, timestamp cache, and others. See the
// ExperimentalPreserveHistory parameter which will uphold these guarantees.
// A RevertRangeRequest specifies a range of keys in which to clear all MVCC
// revisions more recent than some TargetTime from the underlying engine, thus
// reverting the range (from the perspective of an MVCC scan) to that time.
message RevertRangeRequest {
RequestHeader header = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];

// TargetTime specifies a the time to which to "revert" the range to. Any
// versions later than TargetTime will be undone. TargetTime must be higher
// TargetTime specifies a the time to which to "revert" the range by clearing
// any MVCC key with a strictly higher timestamp. TargetTime must be higher
// than the GC Threshold for the replica - so that it is assured that the keys
// for that time are still there — or the request will fail.
util.hlc.Timestamp target_time = 2 [(gogoproto.nullable) = false];

// ExperimentalPreserveHistory will preserve MVCC history by, rather than
// clearing newer versions, deleting them using tombstones or updating them
// back to their original value as of the target time. Long runs of key
// deletions will use an MVCC range tombstone instead. This respects the
// closed timestamp and timestamp cache.
//
// The caller must check storage.CanUseExperimentalMVCCRangeTombstones()
// before enabling this parameter.
//
// This parameter 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.
bool experimental_preserve_history = 5;

bool enable_time_bound_iterator_optimization = 3;

// IgnoreGcThreshold can be set by a caller to ignore the target-time when
Expand Down
2 changes: 0 additions & 2 deletions pkg/roachpb/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,8 @@ func TestFlagCombinations(t *testing.T) {
reqVariants := []Request{
&AddSSTableRequest{SSTTimestampToRequestTimestamp: hlc.Timestamp{Logical: 1}},
&DeleteRangeRequest{Inline: true},
&DeleteRangeRequest{UseExperimentalRangeTombstone: true},
&GetRequest{KeyLocking: lock.Exclusive},
&ReverseScanRequest{KeyLocking: lock.Exclusive},
&RevertRangeRequest{ExperimentalPreserveHistory: true},
&ScanRequest{KeyLocking: lock.Exclusive},
}

Expand Down
2 changes: 0 additions & 2 deletions pkg/storage/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ go_library(
"mvcc_incremental_iterator.go",
"mvcc_key.go",
"mvcc_logical_ops.go",
"mvcc_range_key_iterator.go",
"open.go",
"pebble.go",
"pebble_batch.go",
Expand Down Expand Up @@ -109,7 +108,6 @@ go_test(
"mvcc_incremental_iterator_test.go",
"mvcc_key_test.go",
"mvcc_logical_ops_test.go",
"mvcc_range_key_iterator_test.go",
"mvcc_stats_test.go",
"mvcc_test.go",
"pebble_file_registry_test.go",
Expand Down
Loading