From a9f400695bb37d119732684b2d8e8684cbe11ee6 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Thu, 25 Aug 2022 12:12:29 +0000 Subject: [PATCH] kvserver: add `GlobalMVCCRangeTombstone` testing knob This patch adds a set of testing knobs to enable writing an MVCC range tombstone at the bottom of the keyspace during cluster bootstrapping. This also subsumes the previous mechanism to trigger this from the envvar `COCKROACH_GLOBAL_MVCC_RANGE_TOMBSTONE`, for e.g. roachtests and roachperf benchmarks. Release justification: non-production code changes Release note: None --- pkg/kv/kvclient/rangefeed/BUILD.bazel | 1 - pkg/kv/kvclient/rangefeed/rangefeed.go | 7 +++++-- pkg/kv/kvserver/batcheval/cmd_init_put.go | 3 +-- pkg/kv/kvserver/kvserverbase/BUILD.bazel | 1 - pkg/kv/kvserver/kvserverbase/base.go | 8 -------- pkg/kv/kvserver/kvserverbase/knobs.go | 6 ++++++ pkg/kv/kvserver/store_init.go | 3 +-- pkg/kv/kvserver/testing_knobs.go | 12 ++++++++++++ pkg/server/config.go | 24 +++++++++++++++++++++++ 9 files changed, 49 insertions(+), 16 deletions(-) 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..9b27ab25da32 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,10 @@ 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. to + IgnoreOnDeleteRangeError bool } // ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface. @@ -361,7 +364,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..53c7e5c619ca 100644 --- a/pkg/kv/kvserver/kvserverbase/knobs.go +++ b/pkg/kv/kvserver/kvserverbase/knobs.go @@ -38,6 +38,12 @@ 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 when writing a global MVCC range tombstone across the + // entire user keyspan for testing, in which case we want InitPuts to + // still succeed. + 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