From 9ad36eecabcdafd1da37d1d25472b2265d2cf1ae Mon Sep 17 00:00:00 2001
From: Tobias Grieger <tobias.b.grieger@gmail.com>
Date: Mon, 19 Sep 2022 10:32:52 +0200
Subject: [PATCH] fixup! address comments

Signed-off-by: Tobias Grieger <tobias.b.grieger@gmail.com>
---
 raft/node_util_test.go   |  3 ++-
 raft/raft.go             |  2 +-
 raft/rawnode_test.go     | 48 ++++++++++++++++++++++++++--------------
 raft/tracker/progress.go |  3 +--
 4 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/raft/node_util_test.go b/raft/node_util_test.go
index d344295ad..5093cba6b 100644
--- a/raft/node_util_test.go
+++ b/raft/node_util_test.go
@@ -1,4 +1,4 @@
-// Copyright 2015 The etcd Authors
+// Copyright 2022 The etcd Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -105,5 +105,6 @@ func newNodeTestHarness(ctx context.Context, t *testing.T, cfg *Config, peers ..
 		defer n.Stop()
 		n.run()
 	}()
+	t.Cleanup(n.Stop)
 	return ctx, cancel, &nodeTestHarness{node: n, t: t}
 }
diff --git a/raft/raft.go b/raft/raft.go
index cd4a604e9..258b5cd84 100644
--- a/raft/raft.go
+++ b/raft/raft.go
@@ -647,7 +647,7 @@ func (r *raft) appendEntry(es ...pb.Entry) (accepted bool) {
 		return false
 	}
 	// use latest "last" index after truncate/append
-	li = r.raftLog.append(es...)
+	r.raftLog.append(es...)
 	return true
 }
 
diff --git a/raft/rawnode_test.go b/raft/rawnode_test.go
index 979b6fa8c..e209b3277 100644
--- a/raft/rawnode_test.go
+++ b/raft/rawnode_test.go
@@ -884,17 +884,17 @@ func TestRawNodeStatus(t *testing.T) {
 // TestNodeCommitPaginationAfterRestart. The anomaly here was even worse as the
 // Raft group would forget to apply entries:
 //
-// - node learns that index 11 is committed
-// - nextEnts returns index 1..10 in CommittedEntries (but index 10 already
-//   exceeds maxBytes), which isn't noticed internally by Raft
-// - Commit index gets bumped to 10
-// - the node persists the HardState, but crashes before applying the entries
-// - upon restart, the storage returns the same entries, but `slice` takes a
-//   different code path and removes the last entry.
-// - Raft does not emit a HardState, but when the app calls Advance(), it bumps
-//   its internal applied index cursor to 10 (when it should be 9)
-// - the next Ready asks the app to apply index 11 (omitting index 10), losing a
-//    write.
+//   - node learns that index 11 is committed
+//   - nextEnts returns index 1..10 in CommittedEntries (but index 10 already
+//     exceeds maxBytes), which isn't noticed internally by Raft
+//   - Commit index gets bumped to 10
+//   - the node persists the HardState, but crashes before applying the entries
+//   - upon restart, the storage returns the same entries, but `slice` takes a
+//     different code path and removes the last entry.
+//   - Raft does not emit a HardState, but when the app calls Advance(), it bumps
+//     its internal applied index cursor to 10 (when it should be 9)
+//   - the next Ready asks the app to apply index 11 (omitting index 10), losing a
+//     write.
 func TestRawNodeCommitPaginationAfterRestart(t *testing.T) {
 	s := &ignoreSizeHintMemStorage{
 		MemoryStorage: newTestMemoryStorage(withPeers(1)),
@@ -1133,12 +1133,26 @@ func TestRawNodeConsumeReady(t *testing.T) {
 }
 
 func BenchmarkRawNode(b *testing.B) {
-	b.Run("single-voter", func(b *testing.B) {
-		benchmarkRawNodeImpl(b, 1)
-	})
-	b.Run("two-voters", func(b *testing.B) {
-		benchmarkRawNodeImpl(b, 1, 2)
-	})
+	cases := []struct {
+		name  string
+		peers []uint64
+	}{
+		{
+			name:  "single-voter",
+			peers: []uint64{1},
+		},
+		{
+			name:  "two-voters",
+			peers: []uint64{1, 2},
+		},
+		// You can easily add more cases here.
+	}
+
+	for _, tc := range cases {
+		b.Run(tc.name, func(b *testing.B) {
+			benchmarkRawNodeImpl(b, tc.peers...)
+		})
+	}
 }
 
 func benchmarkRawNodeImpl(b *testing.B, peers ...uint64) {
diff --git a/raft/tracker/progress.go b/raft/tracker/progress.go
index a36e5261a..e37e4b63f 100644
--- a/raft/tracker/progress.go
+++ b/raft/tracker/progress.go
@@ -52,8 +52,7 @@ type Progress struct {
 	// RecentActive is true if the progress is recently active. Receiving any messages
 	// from the corresponding follower indicates the progress is active.
 	// RecentActive can be reset to false after an election timeout.
-	//
-	// TODO(tbg): the leader should always have this set to true.
+	// This is always true on the leader.
 	RecentActive bool
 
 	// ProbeSent is used while this follower is in StateProbe. When ProbeSent is