Skip to content

Commit

Permalink
fixup! address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
tbg committed Sep 20, 2022
1 parent 304e260 commit 9ad36ee
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 21 deletions.
3 changes: 2 additions & 1 deletion raft/node_util_test.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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}
}
2 changes: 1 addition & 1 deletion raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
48 changes: 31 additions & 17 deletions raft/rawnode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions raft/tracker/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9ad36ee

Please sign in to comment.