kvserver: remove ProposeEmptyCommand

We should no longer need it. Also, it introduces the complication
of "empty but ID'ed raft commands", which is annoying. We could
probably change this to propose a truly `nil` command (mimicking
what raft does internally) but it is better to avoid this problem
altogether.

If there truly is a case in which we need to nudge raft, it likely
also exists outside of the `splitDelayHelper`, and the sooner we
find out about it the better.

Extracted from cockroachdb#76126.

Release note: None

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
 				}

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)
 	})
 }