Skip to content

Commit

Permalink
Revert "storage: add experimental MVCC range tombstone primitives"
Browse files Browse the repository at this point in the history 
This reverts commit 9f3ef09.

Release note: None
  • Loading branch information
@erikgrinaker @maryliag
erikgrinaker authored and maryliag committed Feb 28, 2022
1 parent fa38d2a commit 39faa6a
Show file tree
Hide file tree
Showing 21 changed files with 20 additions and 1,355 deletions.
15 changes: 0 additions & 15 deletions pkg/kv/kvserver/rangefeed/task_test.go
View file
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
25 changes: 0 additions & 25 deletions pkg/kv/kvserver/spanset/batch.go
View file
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) {
panic("not implemented")
}

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

// RangeKeys implements SimpleMVCCIterator.
func (i *MVCCIterator) RangeKeys() []storage.MVCCRangeKeyValue {
panic("not implemented")
}

// ComputeStats is part of the storage.MVCCIterator interface.
func (i *MVCCIterator) ComputeStats(
start, end roachpb.Key, nowNanos int64,
Expand Down Expand Up @@ -614,16 +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 {
panic("not implemented")
}

func (s spanSetWriter) ExperimentalClearMVCCRangeKey(rangeKey storage.MVCCRangeKey) error {
panic("not implemented")
}

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
2 changes: 0 additions & 2 deletions pkg/storage/BUILD.bazel
View file
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
92 changes: 1 addition & 91 deletions pkg/storage/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,46 +67,11 @@ type SimpleMVCCIterator interface {
// reverse iteration to forward iteration.
NextKey()
// UnsafeKey returns the same value as Key, but the memory is invalidated on
// the next call to {Next,NextKey,Prev,SeekGE,SeekLT,Close}. If the iterator
// is on a range key only, this returns the start bound of the range key.
// the next call to {Next,NextKey,Prev,SeekGE,SeekLT,Close}.
UnsafeKey() MVCCKey
// UnsafeValue returns the same value as Value, but the memory is
// invalidated on the next call to {Next,NextKey,Prev,SeekGE,SeekLT,Close}.
UnsafeValue() []byte
// HasPointAndRange returns whether the current iterator position has a point
// key and/or a range key. If Valid() returns true, one of these will be true.
// Range keys are only emitted when requested via IterOptions.KeyTypes.
HasPointAndRange() (bool, bool)
// RangeBounds returns the range bounds for the current range key fragment, if
// any. See RangeKeys() for more info on range key fragments.
RangeBounds() (roachpb.Key, roachpb.Key)
// RangeKeys returns all range key fragments (at different timestamps) at the
// current key position. If we are at a point key, it will return all range
// keys that overlap that point key at any timestamp.
//
// For defragmented iteration, use MVCCRangeKeyIterator instead. Fragmented
// iteration is primarily useful in two cases:
//
// - To iterate over point keys while accessing overlapping range keys
// (e.g. to determine if it is hidden by a range tombstone).
//
// - For partial iteration with later resumption, e.g. Export requests with
// byte limits that have to return point and range key data for a partial
// key span and then resume from that point in a later request.
//
// Range keys are fragmented by Pebble such that all overlapping range keys
// between two fragment bounds form a "stack" of range key fragments at
// different timestamps. Fragmentation is desirable at the storage layer to
// store range keys across SSTs and CRDB ranges without incurring
// cross-SST/range access costs. Stacking is desirable to easily see all range
// keys that overlap with a given point, and to translate range keys from the
// 2D MVCC keyspan to the 1D Pebble keyspan.
//
// Range keys will also split and merge along with CRDB ranges, can be
// partially removed by GC, and may be truncated by iterator bounds.
//
// TODO(erikgrinaker): Write a tech note on range keys and link it here.
RangeKeys() []MVCCRangeKeyValue
}

// IteratorStats is returned from {MVCCIterator,EngineIterator}.Stats.
Expand Down Expand Up @@ -344,27 +309,8 @@ type IterOptions struct {
// use such an iterator is to use it in concert with an iterator without
// timestamp hints, as done by MVCCIncrementalIterator.
MinTimestampHint, MaxTimestampHint hlc.Timestamp
// KeyTypes specifies the types of keys to surface: point and/or range keys.
// Use HasPointAndRange() to determine which key type is present at a given
// iterator position, and RangeBounds() and RangeKeys() to access range keys.
// Defaults to IterKeyTypePointsOnly. For more info, see RangeKeys().
KeyTypes IterKeyType
}

// IterKeyType configures which types of keys an iterator should surface.
//
// TODO(erikgrinaker): Combine this with MVCCIterKind somehow.
type IterKeyType = pebble.IterKeyType

const (
// IterKeyTypePointsOnly iterates over point keys only.
IterKeyTypePointsOnly = pebble.IterKeyTypePointsOnly
// IterKeyTypePointsAndRanges iterates over both point and range keys.
IterKeyTypePointsAndRanges = pebble.IterKeyTypePointsAndRanges
// IterKeyTypeRangesOnly iterates over only range keys.
IterKeyTypeRangesOnly = pebble.IterKeyTypeRangesOnly
)

// MVCCIterKind is used to inform Reader about the kind of iteration desired
// by the caller.
type MVCCIterKind int
Expand Down Expand Up @@ -629,42 +575,6 @@ type Writer interface {
// returns.
ClearIterRange(iter MVCCIterator, start, end roachpb.Key) error

// ExperimentalClearMVCCRangeKey deletes an MVCC range key from start
// (inclusive) to end (exclusive) at the given timestamp. For any range key
// that straddles the start and end boundaries, only the segments within the
// boundaries will be cleared. Clears are idempotent.
//
// This method is primarily intended for MVCC garbage collection and similar
// internal use. It mutates MVCC history, and does not check for intents or
// other conflicts.
//
// TODO(erikgrinaker): We'll likely need another method that calls through to
// Pebble's RangeKeyDelete(), which removes all range keys in a span. This
// will be used e.g. when removing replicas.
//
// This method is EXPERIMENTAL: range keys are under active development, and
// have severe limitations including being ignored by all KV and MVCC APIs and
// only being stored in memory.
ExperimentalClearMVCCRangeKey(rangeKey MVCCRangeKey) error

// ExperimentalPutMVCCRangeKey writes a value to an MVCC range key. It is
// currently only used for range tombstones, which have a value of nil. Range
// keys exist separately from point keys in Pebble, and must be accessed via
// specialized iterator options and methods -- see e.g. IterOptions.KeyTypes,
// SimpleMVCCIterator.RangeKeys(), and MVCCRangeKeyIterator.
//
// A range key does not have a distinct identity, but should be considered a
// key continuum. They can be fragmented by Pebble as overlapping keys are
// written or removed, split/merged along with CRDB ranges, partially removed
// with ExperimentalClearMVCCRangeKey, and truncated during bounded iteration.
//
// TODO(erikgrinaker): Write a tech note on range keys and link it here.
//
// This method is EXPERIMENTAL: range keys are under active development, and
// have severe limitations including being ignored by all KV and MVCC APIs and
// only being stored in memory.
ExperimentalPutMVCCRangeKey(MVCCRangeKey, []byte) error

// Merge is a high-performance write operation used for values which are
// accumulated over several writes. Multiple values can be merged
// sequentially into a single key; a subsequent read will return a "merged"
Expand Down
15 changes: 0 additions & 15 deletions pkg/storage/intent_interleaving_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,21 +715,6 @@ func (i *intentInterleavingIter) Value() []byte {
return i.iter.Value()
}

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

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

// RangeKeys implements SimpleMVCCIterator.
func (i *intentInterleavingIter) RangeKeys() []MVCCRangeKeyValue {
panic("not implemented")
}

func (i *intentInterleavingIter) Close() {
i.iter.Close()
i.intentIter.Close()
Expand Down
16 changes: 0 additions & 16 deletions pkg/storage/multi_iterator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"bytes"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
)

const invalidIdxSentinel = -1
Expand Down Expand Up @@ -93,21 +92,6 @@ func (f *multiIterator) UnsafeValue() []byte {
return f.iters[f.currentIdx].UnsafeValue()
}

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

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

// RangeKeys implements SimpleMVCCIterator.
func (f *multiIterator) RangeKeys() []MVCCRangeKeyValue {
panic("not implemented")
}

// Next advances the iterator to the next key/value in the iteration. After this
// call, Valid() will be true if the iterator was not positioned at the last
// key.
Expand Down
34 changes: 0 additions & 34 deletions pkg/storage/mvcc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,6 @@ type MVCCKeyValue struct {
Value []byte
}

// MVCCRangeKeyValue represents a ranged key/value pair.
type MVCCRangeKeyValue struct {
Key MVCCRangeKey
Value []byte
}

// optionalValue represents an optional roachpb.Value. It is preferred
// over a *roachpb.Value to avoid the forced heap allocation.
type optionalValue struct {
Expand Down Expand Up @@ -2208,34 +2202,6 @@ func MVCCDeleteRange(
return keys, res.ResumeSpan, res.NumKeys, nil
}

// ExperimentalMVCCDeleteRangeUsingTombstone deletes the given MVCC keyspan at
// the given timestamp using a range tombstone (rather than point tombstones).
// This operation is non-transactional, but will check for existing intents and
// return a WriteIntentError containing up to maxIntents intents.
//
// This method is EXPERIMENTAL: range keys are under active development, and
// have severe limitations including being ignored by all KV and MVCC APIs and
// only being stored in memory.
//
// TODO(erikgrinaker): Needs handling of conflicts (e.g. WriteTooOldError),
// MVCCStats, and tests.
func ExperimentalMVCCDeleteRangeUsingTombstone(
ctx context.Context,
rw ReadWriter,
ms *enginepb.MVCCStats,
startKey, endKey roachpb.Key,
timestamp hlc.Timestamp,
maxIntents int64,
) error {
if intents, err := ScanIntents(ctx, rw, startKey, endKey, maxIntents, 0); err != nil {
return err
} else if len(intents) > 0 {
return &roachpb.WriteIntentError{Intents: intents}
}
return rw.ExperimentalPutMVCCRangeKey(MVCCRangeKey{
StartKey: startKey, EndKey: endKey, Timestamp: timestamp}, nil)
}

func recordIteratorStats(traceSpan *tracing.Span, iteratorStats IteratorStats) {
stats := iteratorStats.Stats
if traceSpan != nil {
Expand Down
Loading

0 comments on commit 39faa6a

Please sign in to comment.