Revert "batcheval: add range tombstone support for DeleteRange"

This reverts commit f07cfd6.

Release note: None

pkg/clusterversion/cockroach_versions.go

@@ -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.
@@ -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},

pkg/kv/batch.go

@@ -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{}) {

pkg/kv/db.go

@@ -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

pkg/kv/kvserver/batcheval/cmd_delete_range.go

@@ -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() {
@@ -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

pkg/kv/kvserver/spanset/batch.go

@@ -178,17 +178,17 @@ func (i *MVCCIterator) UnsafeValue() []byte {
 
 // HasPointAndRange implements SimpleMVCCIterator.
 func (i *MVCCIterator) HasPointAndRange() (bool, bool) {
-	return i.i.HasPointAndRange()
+	panic("not implemented")
 }
 
 // RangeBounds implements SimpleMVCCIterator.
 func (i *MVCCIterator) RangeBounds() (roachpb.Key, roachpb.Key) {
-	return i.i.RangeBounds()
+	panic("not implemented")
 }
 
 // RangeKeys implements SimpleMVCCIterator.
 func (i *MVCCIterator) RangeKeys() []storage.MVCCRangeKeyValue {
-	return i.i.RangeKeys()
+	panic("not implemented")
 }
 
 // ComputeStats is part of the storage.MVCCIterator interface.
@@ -617,17 +617,11 @@ func (s spanSetWriter) ClearIterRange(iter storage.MVCCIterator, start, end roac
 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)
+	panic("not implemented")
 }
 
 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)
+	panic("not implemented")
 }
 
 func (s spanSetWriter) Merge(key storage.MVCCKey, value []byte) error {

pkg/roachpb/api.go

@@ -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

pkg/roachpb/api.proto

@@ -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()

pkg/roachpb/api_test.go

@@ -318,7 +318,6 @@ 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},
 		&ScanRequest{KeyLocking: lock.Exclusive},

pkg/storage/mvcc.go

@@ -20,12 +20,10 @@ import (
 	"sync"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/uncertainty"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -86,19 +84,6 @@ var rocksdbConcurrency = envutil.EnvOrDefaultInt(
 		return max
 	}())
 
-// CanUseExperimentalMVCCRangeTombstones returns true if MVCC range tombstones
-// are enabled. Callers must check this before using range tombstones.
-//
-// These are 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 CanUseExperimentalMVCCRangeTombstones(ctx context.Context, st *cluster.Settings) bool {
-	// TODO(erikgrinaker): Consider using a cluster setting rather than an
-	// environment variable once range tombstones are fully implemented.
-	return st.Version.IsActive(ctx, clusterversion.ExperimentalMVCCRangeTombstones) &&
-		envutil.EnvOrDefaultBool("COCKROACH_EXPERIMENTAL_MVCC_RANGE_TOMBSTONES", false)
-}
-
 // MakeValue returns the inline value.
 func MakeValue(meta enginepb.MVCCMetadata) roachpb.Value {
 	return roachpb.Value{RawBytes: meta.RawBytes}