From 31947b8285b9ccac416ae8f5b929f385006603cf 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 | 9 +++++++-- 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 | 5 +++++ pkg/kv/kvserver/store_init.go | 3 +-- pkg/kv/kvserver/testing_knobs.go | 12 ++++++++++++ pkg/server/config.go | 24 +++++++++++++++++++++++ 9 files changed, 50 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..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