From 01062335fdf329752e60da1d33be1fcbd80a622d Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Mon, 29 May 2023 17:59:44 +0000 Subject: [PATCH 1/2] kvserver: move `COCKROACH_DISABLE_LEADER_FOLLOWS_LEASEHOLDER` Epic: none Release note: None --- pkg/kv/kvserver/replica_raft.go | 5 +++++ pkg/kv/kvserver/store.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/kv/kvserver/replica_raft.go b/pkg/kv/kvserver/replica_raft.go index d7dd421e8db7..912dfa22086e 100644 --- a/pkg/kv/kvserver/replica_raft.go +++ b/pkg/kv/kvserver/replica_raft.go @@ -35,6 +35,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/buildutil" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/humanizeutil" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -69,6 +70,10 @@ var ( // ~16 bytes, so Pebble point deletion batches will be bounded at ~1.6MB. raftLogTruncationClearRangeThreshold = kvpb.RaftIndex(util.ConstantWithMetamorphicTestRange( "raft-log-truncation-clearrange-threshold", 100000 /* default */, 1 /* min */, 1e6 /* max */)) + + // raftDisableLeaderFollowsLeaseholder disables lease/leader colocation. + raftDisableLeaderFollowsLeaseholder = envutil.EnvOrDefaultBool( + "COCKROACH_DISABLE_LEADER_FOLLOWS_LEASEHOLDER", false) ) func makeIDKey() kvserverbase.CmdIDKey { diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 311782130830..2a82660f7bab 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -1226,7 +1226,7 @@ func (sc *StoreConfig) SetDefaults(numStores int) { if sc.RaftEntryCacheSize == 0 { sc.RaftEntryCacheSize = defaultRaftEntryCacheSize } - if envutil.EnvOrDefaultBool("COCKROACH_DISABLE_LEADER_FOLLOWS_LEASEHOLDER", false) { + if raftDisableLeaderFollowsLeaseholder { sc.TestingKnobs.DisableLeaderFollowsLeaseholder = true sc.TestingKnobs.AllowLeaseRequestProposalsWhenNotLeader = true // otherwise lease requests fail } From f290b126a571f1585b40c160e978d2e888f97f07 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Mon, 29 May 2023 18:00:08 +0000 Subject: [PATCH 2/2] kvserver: add DisableQuiescence testing knob This patch turns `COCKROACH_DISABLE_QUIESCENCE` into a testing knob, for better control in tests. Epic: none Release note: None --- pkg/kv/kvserver/raft_log_queue_test.go | 22 +++++++++------------- pkg/kv/kvserver/replica_raft_quiesce.go | 10 +++++----- pkg/kv/kvserver/store.go | 3 +++ pkg/kv/kvserver/testing_knobs.go | 3 +++ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/pkg/kv/kvserver/raft_log_queue_test.go b/pkg/kv/kvserver/raft_log_queue_test.go index a59aeb94e498..c4ebd6df95f6 100644 --- a/pkg/kv/kvserver/raft_log_queue_test.go +++ b/pkg/kv/kvserver/raft_log_queue_test.go @@ -449,22 +449,18 @@ func TestNewTruncateDecision(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - // Unquiescing can add spurious empty log entries. Just disable it. - testingDisableQuiescence = true - defer func() { - testingDisableQuiescence = false - }() - ctx := context.Background() stopper := stop.NewStopper() defer stopper.Stop(ctx) - store, _ := createTestStore(ctx, t, - testStoreOpts{ - // This test was written before test stores could start with more than one - // range and was not adapted. - createSystemRanges: false, - }, - stopper) + + opts := testStoreOpts{ + // This test was written before test stores could start with more than one + // range and was not adapted. + createSystemRanges: false, + } + cfg := TestStoreConfig(nil /* clock */) + cfg.TestingKnobs.DisableQuiescence = true // quiescence adds spurious empty log entries + store := createTestStoreWithConfig(ctx, t, stopper, opts, &cfg) store.SetRaftLogQueueActive(false) r, err := store.GetReplica(1) diff --git a/pkg/kv/kvserver/replica_raft_quiesce.go b/pkg/kv/kvserver/replica_raft_quiesce.go index d8cf55e0a5a8..6840a0a2d441 100644 --- a/pkg/kv/kvserver/replica_raft_quiesce.go +++ b/pkg/kv/kvserver/replica_raft_quiesce.go @@ -34,8 +34,8 @@ import ( // e.g. on every tick. var quiesceAfterTicks = envutil.EnvOrDefaultInt("COCKROACH_QUIESCE_AFTER_TICKS", 6) -// testingDisableQuiescence disables replica quiescence. -var testingDisableQuiescence = envutil.EnvOrDefaultBool("COCKROACH_DISABLE_QUIESCENCE", false) +// raftDisableQuiescence disables raft quiescence. +var raftDisableQuiescence = envutil.EnvOrDefaultBool("COCKROACH_DISABLE_QUIESCENCE", false) func (r *Replica) quiesceLocked(ctx context.Context, lagging laggingReplicaSet) { if !r.mu.quiescent { @@ -195,6 +195,9 @@ func (r *Replica) canUnquiesceRLocked() bool { func (r *Replica) maybeQuiesceRaftMuLockedReplicaMuLocked( ctx context.Context, leaseStatus kvserverpb.LeaseStatus, livenessMap livenesspb.IsLiveMap, ) bool { + if r.store.cfg.TestingKnobs.DisableQuiescence { + return false + } status, lagging, ok := shouldReplicaQuiesce(ctx, r, leaseStatus, livenessMap, r.mu.pausedFollowers) if !ok { return false @@ -291,9 +294,6 @@ func shouldReplicaQuiesce( livenessMap livenesspb.IsLiveMap, pausedFollowers map[roachpb.ReplicaID]struct{}, ) (*raftSparseStatus, laggingReplicaSet, bool) { - if testingDisableQuiescence { - return nil, nil, false - } if !q.isRaftLeaderRLocked() { // fast path if log.V(4) { log.Infof(ctx, "not quiescing: not leader") diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 2a82660f7bab..29cefb6745e3 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -1230,6 +1230,9 @@ func (sc *StoreConfig) SetDefaults(numStores int) { sc.TestingKnobs.DisableLeaderFollowsLeaseholder = true sc.TestingKnobs.AllowLeaseRequestProposalsWhenNotLeader = true // otherwise lease requests fail } + if raftDisableQuiescence { + sc.TestingKnobs.DisableQuiescence = true + } } // GetStoreConfig exposes the config used for this store. diff --git a/pkg/kv/kvserver/testing_knobs.go b/pkg/kv/kvserver/testing_knobs.go index e5da7381bba4..357f2dbaa8df 100644 --- a/pkg/kv/kvserver/testing_knobs.go +++ b/pkg/kv/kvserver/testing_knobs.go @@ -217,6 +217,9 @@ type StoreTestingKnobs struct { DisableConsistencyQueue bool // DisableScanner disables the replica scanner. DisableScanner bool + // DisableQuiescence disables replica quiescence. This can also be + // set via COCKROACH_DISABLE_QUIESCENCE. + DisableQuiescence bool // DisableLeaderFollowsLeaseholder disables attempts to transfer raft // leadership when it diverges from the range's leaseholder. This can // also be set via COCKROACH_DISABLE_LEADER_FOLLOWS_LEASEHOLDER.