diff --git a/pkg/kv/kvserver/split_delay_helper.go b/pkg/kv/kvserver/split_delay_helper.go index 1258426f3dfe..5cf3c511eac9 100644 --- a/pkg/kv/kvserver/split_delay_helper.go +++ b/pkg/kv/kvserver/split_delay_helper.go @@ -17,7 +17,6 @@ import ( "strings" "time" - "github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "go.etcd.io/etcd/raft/v3" @@ -26,7 +25,6 @@ import ( type splitDelayHelperI interface { RaftStatus(context.Context) (roachpb.RangeID, *raft.Status) - ProposeEmptyCommand(ctx context.Context) MaxTicks() int TickDuration() time.Duration Sleep(context.Context, time.Duration) @@ -58,20 +56,6 @@ func (sdh *splitDelayHelper) Sleep(ctx context.Context, dur time.Duration) { } } -func (sdh *splitDelayHelper) ProposeEmptyCommand(ctx context.Context) { - r := (*Replica)(sdh) - r.raftMu.Lock() - _ = r.withRaftGroup(true /* campaignOnWake */, func(rawNode *raft.RawNode) (bool, error) { - // NB: intentionally ignore the error (which can be ErrProposalDropped - // when there's an SST inflight). - data := kvserverbase.EncodeRaftCommand(kvserverbase.RaftVersionStandard, makeIDKey(), nil) - _ = rawNode.Propose(data) - // NB: we need to unquiesce as the group might be quiesced. - return true /* unquiesceAndWakeLeader */, nil - }) - r.raftMu.Unlock() -} - func (sdh *splitDelayHelper) MaxTicks() int { // There is a related mechanism regarding snapshots and splits that is worth // pointing out here: Incoming MsgApp (see the _ assignment below) are @@ -177,17 +161,6 @@ func maybeDelaySplitToAvoidSnapshot(ctx context.Context, sdh splitDelayHelperI) if slept < tickDur { // We don't want to delay splits for a follower who hasn't responded within a tick. problems = append(problems, fmt.Sprintf("r%d/%d inactive", rangeID, replicaID)) - if i == 1 { - // Propose an empty command which works around a Raft bug that can - // leave a follower in ProgressStateProbe even though it has caught - // up. - // - // We have long picked up a fix[1] for the bug, but there might be similar - // issues we're not aware of and this doesn't hurt, so leave it in for now. - // - // [1]: https://github.com/etcd-io/etcd/commit/bfaae1ba462c91aaf149a285b8d2369807044f71 - sdh.ProposeEmptyCommand(ctx) - } } continue } diff --git a/pkg/kv/kvserver/split_delay_helper_test.go b/pkg/kv/kvserver/split_delay_helper_test.go index 728dc5b63a6a..8cd05638a303 100644 --- a/pkg/kv/kvserver/split_delay_helper_test.go +++ b/pkg/kv/kvserver/split_delay_helper_test.go @@ -30,16 +30,13 @@ type testSplitDelayHelper struct { raftStatus *raft.Status sleep func() - slept time.Duration - emptyProposed int + slept time.Duration } func (h *testSplitDelayHelper) RaftStatus(context.Context) (roachpb.RangeID, *raft.Status) { return h.rangeID, h.raftStatus } -func (h *testSplitDelayHelper) ProposeEmptyCommand(ctx context.Context) { - h.emptyProposed++ -} + func (h *testSplitDelayHelper) MaxTicks() int { return h.numAttempts } @@ -135,7 +132,6 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) { // We try to wake up the follower once, but then give up on it. assert.Equal(t, "; delayed by 1.3s to resolve: r1/2 inactive", s) assert.Less(t, int64(h.slept), int64(2*h.TickDuration())) - assert.Equal(t, 1, h.emptyProposed) }) for _, state := range []tracker.StateType{tracker.StateProbe, tracker.StateSnapshot} { @@ -159,7 +155,6 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) { s := maybeDelaySplitToAvoidSnapshot(ctx, h) assert.Equal(t, "; delayed by 5.5s to resolve: replica r1/2 not caught up: "+ state.String()+" match=0 next=0 paused (without success)", s) - assert.Equal(t, 0, h.emptyProposed) }) } @@ -176,7 +171,6 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) { s := maybeDelaySplitToAvoidSnapshot(ctx, h) assert.Equal(t, "", s) assert.EqualValues(t, 0, h.slept) - assert.Equal(t, 0, h.emptyProposed) }) t.Run("becomes-replicating", func(t *testing.T) { @@ -199,6 +193,5 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) { } s := maybeDelaySplitToAvoidSnapshot(ctx, h) assert.Equal(t, "; delayed by 2.5s to resolve: replica r1/2 not caught up: StateProbe match=0 next=0", s) - assert.EqualValues(t, 0, h.emptyProposed) }) }