sql/logictest: add metamorphic param for global MVCC range tombstone

pkg/kv/kvclient/rangefeed/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
         "//pkg/keys",
         "//pkg/kv",
         "//pkg/kv/kvclient/kvcoord",
-        "//pkg/kv/kvserver/kvserverbase",
         "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/settings/cluster",

pkg/kv/kvclient/rangefeed/rangefeed.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
@@ -83,6 +82,12 @@ type TestingKnobs struct {
 
 	// OnRangefeedRestart is called when a rangefeed restarts.
 	OnRangefeedRestart func()
+
+	// IgnoreOnDeleteRangeError will ignore any errors where a DeleteRange event
+	// is emitted without an OnDeleteRange handler. This can be used e.g. with
+	// StoreTestingKnobs.GlobalMVCCRangeTombstone, to prevent the global tombstone
+	// causing rangefeed errors for consumers who don't expect it.
+	IgnoreOnDeleteRangeError bool
 }
 
 // ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface.
@@ -361,7 +366,7 @@ func (f *RangeFeed) processEvents(
 				f.onSSTable(ctx, ev.SST, ev.RegisteredSpan)
 			case ev.DeleteRange != nil:
 				if f.onDeleteRange == nil {
-					if kvserverbase.GlobalMVCCRangeTombstoneForTesting {
+					if f.knobs.IgnoreOnDeleteRangeError {
 						continue
 					}
 					return errors.AssertionFailedf(

pkg/kv/kvserver/batcheval/cmd_init_put.go

@@ -14,7 +14,6 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 )
@@ -33,7 +32,7 @@ func InitPut(
 	args := cArgs.Args.(*roachpb.InitPutRequest)
 	h := cArgs.Header
 
-	if args.FailOnTombstones && kvserverbase.GlobalMVCCRangeTombstoneForTesting {
+	if args.FailOnTombstones && cArgs.EvalCtx.EvalKnobs().DisableInitPutFailOnTombstones {
 		args.FailOnTombstones = false
 	}
 

pkg/kv/kvserver/kvserverbase/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
         "//pkg/kv/kvserver/kvserverpb",
         "//pkg/roachpb",
         "//pkg/settings",
-        "//pkg/util/envutil",
         "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/quotapool",

pkg/kv/kvserver/kvserverbase/base.go

@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
-	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/redact"
@@ -219,10 +218,3 @@ var SplitByLoadMergeDelay = settings.RegisterDurationSetting(
 // MaxCommandSizeDefault is the default for the kv.raft.command.max_size
 // cluster setting.
 const MaxCommandSizeDefault = 64 << 20
-
-// GlobalMVCCRangeTombstoneForTesting will write an MVCC range tombstone at the
-// bottom of the SQL table data keyspace during cluster bootstrapping, for
-// performance and correctness testing. This shouldn't affect data written above
-// it, but activates range key-specific code paths in the storage layer.
-var GlobalMVCCRangeTombstoneForTesting = envutil.EnvOrDefaultBool(
-	"COCKROACH_GLOBAL_MVCC_RANGE_TOMBSTONE", false)

pkg/kv/kvserver/kvserverbase/knobs.go

@@ -38,6 +38,11 @@ type BatchEvalTestingKnobs struct {
 	// default, this is not allowed because it is unsafe. See cmd_gc.go for an
 	// explanation of why.
 	AllowGCWithNewThresholdAndKeys bool
+
+	// DisableInitPutFailOnTombstones disables FailOnTombstones for InitPut. This
+	// is useful together with e.g. StoreTestingKnobs.GlobalMVCCRangeTombstone,
+	// where we still want InitPut to succeed on top of the range tombstone.
+	DisableInitPutFailOnTombstones bool
 }
 
 // IntentResolverTestingKnobs contains testing helpers that are used during

pkg/kv/kvserver/store_init.go

@@ -14,7 +14,6 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/keys"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
@@ -204,7 +203,7 @@ func WriteInitialClusterData(
 
 		// If requested, write an MVCC range tombstone at the bottom of the
 		// keyspace, for performance and correctness testing.
-		if kvserverbase.GlobalMVCCRangeTombstoneForTesting {
+		if knobs.GlobalMVCCRangeTombstone {
 			if err := writeGlobalMVCCRangeTombstone(ctx, batch, desc, now.Prev()); err != nil {
 				return err
 			}

pkg/kv/kvserver/testing_knobs.go

@@ -422,6 +422,18 @@ type StoreTestingKnobs struct {
 
 	// EnqueueReplicaInterceptor intercepts calls to `store.Enqueue()`.
 	EnqueueReplicaInterceptor func(queueName string, replica *Replica)
+
+	// GlobalMVCCRangeTombstone will write a global MVCC range tombstone across
+	// the entire user keyspace during cluster bootstrapping. This will be written
+	// below all other data, and thus won't affect query results, but it does
+	// activate MVCC range tombstone code paths in the storage layer for testing.
+	//
+	// This must typically be combined with the following knobs to prevent
+	// various components choking on the range tombstone:
+	//
+	// - rangefeed.TestingKnobs.IgnoreOnDeleteRangeError
+	// - kvserverbase.BatchEvalTestingKnobs.DisableInitPutFailOnTombstones
+	GlobalMVCCRangeTombstone bool
 }
 
 // ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface.

pkg/server/config.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/docs"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/server/status"
@@ -224,9 +225,32 @@ func MakeBaseConfig(st *cluster.Settings, tr *tracing.Tracer) BaseConfig {
 	// in a tag that is prefixed with "nsql".
 	baseCfg.AmbientCtx.AddLogTag("n", baseCfg.IDContainer)
 	baseCfg.InitDefaults()
+	baseCfg.InitTestingKnobs()
+
 	return baseCfg
 }
 
+// InitTestingKnobs sets up any testing knobs based on e.g. envvars.
+func (cfg *BaseConfig) InitTestingKnobs() {
+	// If requested, write an MVCC range tombstone at the bottom of the SQL table
+	// data keyspace during cluster bootstrapping, for performance and correctness
+	// testing. This shouldn't affect data written above it, but activates range
+	// key-specific code paths in the storage layer. We'll also have to tweak
+	// rangefeeds and batcheval to not choke on it.
+	if envutil.EnvOrDefaultBool("COCKROACH_GLOBAL_MVCC_RANGE_TOMBSTONE", false) {
+		if cfg.TestingKnobs.Store == nil {
+			cfg.TestingKnobs.Store = &kvserver.StoreTestingKnobs{}
+		}
+		if cfg.TestingKnobs.RangeFeed == nil {
+			cfg.TestingKnobs.RangeFeed = &rangefeed.TestingKnobs{}
+		}
+		storeKnobs := cfg.TestingKnobs.Store.(*kvserver.StoreTestingKnobs)
+		storeKnobs.GlobalMVCCRangeTombstone = true
+		storeKnobs.EvalKnobs.DisableInitPutFailOnTombstones = true
+		cfg.TestingKnobs.RangeFeed.(*rangefeed.TestingKnobs).IgnoreOnDeleteRangeError = true
+	}
+}
+
 // Config holds the parameters needed to set up a combined KV and SQL server.
 type Config struct {
 	BaseConfig

pkg/sql/logictest/BUILD.bazel

@@ -29,7 +29,9 @@ go_library(
         "//pkg/base",
         "//pkg/cloud/externalconn/providers",
         "//pkg/clusterversion",
+        "//pkg/kv/kvclient/rangefeed",
         "//pkg/kv/kvserver",
+        "//pkg/kv/kvserver/kvserverbase",
         "//pkg/roachpb",
         "//pkg/security/username",
         "//pkg/server",
@@ -56,6 +58,7 @@ go_library(
         "//pkg/testutils/skip",
         "//pkg/testutils/sqlutils",
         "//pkg/upgrade",
+        "//pkg/util",
         "//pkg/util/envutil",
         "//pkg/util/log",
         "//pkg/util/randutil",

pkg/sql/logictest/logic.go

@@ -40,7 +40,9 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	_ "github.com/cockroachdb/cockroach/pkg/cloud/externalconn/providers" // imported to register ExternalConnection providers
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server"
@@ -67,6 +69,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/upgrade"
+	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
@@ -530,6 +533,13 @@ var (
 		"declarative-corpus", "",
 		"enables generation and storage of a declarative schema changer	corpus",
 	)
+
+	// globalMVCCRangeTombstone will write a global MVCC range tombstone across
+	// the entire user keyspace during cluster bootstrapping. This should not
+	// semantically affect the test data written above it, but will activate MVCC
+	// range tombstone code paths in the storage layer for testing.
+	globalMVCCRangeTombstone = util.ConstantWithMetamorphicTestBool(
+		"logictest-global-mvcc-range-tombstone", false)
 )
 
 const queryRewritePlaceholderPrefix = "__async_query_rewrite_placeholder"
@@ -1186,6 +1196,12 @@ func (t *logicTest) newCluster(
 	// when run with fakedist-disk config, so we'll use a larger limit here.
 	// There isn't really a downside to doing so.
 	tempStorageDiskLimit := int64(512 << 20) /* 512 MiB */
+	// MVCC range tombstones are only available in 22.2 or newer.
+	enableGlobalMVCCRangeTombstone := globalMVCCRangeTombstone &&
+		(t.cfg.BootstrapVersion.Equal(roachpb.Version{}) ||
+			!t.cfg.BootstrapVersion.Less(roachpb.Version{Major: 22, Minor: 2})) &&
+		(t.cfg.BinaryVersion.Equal(roachpb.Version{}) ||
+			!t.cfg.BinaryVersion.Less(roachpb.Version{Major: 22, Minor: 2}))
 
 	params := base.TestClusterArgs{
 		ServerArgs: base.TestServerArgs{
@@ -1197,7 +1213,11 @@ func (t *logicTest) newCluster(
 			Knobs: base.TestingKnobs{
 				Store: &kvserver.StoreTestingKnobs{
 					// The consistency queue makes a lot of noisy logs during logic tests.
-					DisableConsistencyQueue: true,
+					DisableConsistencyQueue:  true,
+					GlobalMVCCRangeTombstone: enableGlobalMVCCRangeTombstone,
+					EvalKnobs: kvserverbase.BatchEvalTestingKnobs{
+						DisableInitPutFailOnTombstones: enableGlobalMVCCRangeTombstone,
+					},
 				},
 				SQLEvalContext: &eval.TestingKnobs{
 					AssertBinaryExprReturnTypes:     true,
@@ -1217,6 +1237,9 @@ func (t *logicTest) newCluster(
 				SQLDeclarativeSchemaChanger: &scexec.TestingKnobs{
 					BeforeStage: corpusCollectionCallback,
 				},
+				RangeFeed: rangefeed.TestingKnobs{
+					IgnoreOnDeleteRangeError: enableGlobalMVCCRangeTombstone,
+				},
 			},
 			ClusterName:   "testclustername",
 			ExternalIODir: t.sharedIODir,