diff --git a/pkg/kv/kvclient/rangefeed/BUILD.bazel b/pkg/kv/kvclient/rangefeed/BUILD.bazel index c2206f4d9f69..635b1bd3112b 100644 --- a/pkg/kv/kvclient/rangefeed/BUILD.bazel +++ b/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", diff --git a/pkg/kv/kvclient/rangefeed/rangefeed.go b/pkg/kv/kvclient/rangefeed/rangefeed.go index eda29adfdca1..22e90735353f 100644 --- a/pkg/kv/kvclient/rangefeed/rangefeed.go +++ b/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( diff --git a/pkg/kv/kvserver/batcheval/cmd_init_put.go b/pkg/kv/kvserver/batcheval/cmd_init_put.go index 6c2c432cc1c6..30ebb1e2451a 100644 --- a/pkg/kv/kvserver/batcheval/cmd_init_put.go +++ b/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 } diff --git a/pkg/kv/kvserver/kvserverbase/BUILD.bazel b/pkg/kv/kvserver/kvserverbase/BUILD.bazel index a424377a5adb..09d46d29899d 100644 --- a/pkg/kv/kvserver/kvserverbase/BUILD.bazel +++ b/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", diff --git a/pkg/kv/kvserver/kvserverbase/base.go b/pkg/kv/kvserver/kvserverbase/base.go index 35c55dca2cfe..b2f8df0c683b 100644 --- a/pkg/kv/kvserver/kvserverbase/base.go +++ b/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) diff --git a/pkg/kv/kvserver/kvserverbase/knobs.go b/pkg/kv/kvserver/kvserverbase/knobs.go index 95d50a800d43..7e420b9d167f 100644 --- a/pkg/kv/kvserver/kvserverbase/knobs.go +++ b/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 diff --git a/pkg/kv/kvserver/store_init.go b/pkg/kv/kvserver/store_init.go index 7d594c6944b3..854daae6c465 100644 --- a/pkg/kv/kvserver/store_init.go +++ b/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 } diff --git a/pkg/kv/kvserver/testing_knobs.go b/pkg/kv/kvserver/testing_knobs.go index ce473fc30341..2763515b0774 100644 --- a/pkg/kv/kvserver/testing_knobs.go +++ b/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. diff --git a/pkg/server/config.go b/pkg/server/config.go index ba4a4d0573f8..f8ff1467366a 100644 --- a/pkg/server/config.go +++ b/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 diff --git a/pkg/sql/logictest/BUILD.bazel b/pkg/sql/logictest/BUILD.bazel index aac8d57a4a43..55b4a85c5fac 100644 --- a/pkg/sql/logictest/BUILD.bazel +++ b/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", diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index e274ac7bfe4e..5712d50595d7 100644 --- a/pkg/sql/logictest/logic.go +++ b/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,