Scanning the uncommitted portion of the raft log to determine whether
there are any pending config changes can be expensive. In
cockroachdb/cockroach#18601, we've seen that a new leader can spend so
much time scanning its log post-election that it fails to send
its first heartbeats in time to prevent a second election from
starting immediately.
Instead of tracking whether a pending config change exists with a
boolean, this commit tracks the latest log index at which a pending
config change *could* exist. This is a less expensive solution to
the problem, and the impact of false positives should be minimal since
a newly-elected leader should be able to quickly commit the tail of
its log.
TestRecvMsgPreVote was intended to be introduced in
github.com/coreos/etcd/pull/6624 but was uncapitalized (search for
testRecvMsgPreVote instead) and then subsequently removed due to it
being unused.
TestNodeWithSmallerTermCanCompleteElection tests the scenario where a
node that has been partitioned away (and fallen behind) rejoins the
cluster at about the same time the leader node gets partitioned away.
Previously the cluster would come to a standstill when run with PreVote
enabled.
When responding to Msg{Pre,}Vote messages we now include the term from
the message, not the local term. To see why consider the case where a
single node was previously partitioned away and it's local term is now
of date. If we include the local term (recall that for pre-votes we
don't update the local term), the (pre-)campaigning node on the other
end will proceed to ignore the message (it ignores all out of date
messages).
The term in the original message and current local term are the same in
the case of regular votes, but different for pre-votes.
NB: Had to change TestRecvMsgVote to include pb.Message.Term when
sending MsgVote messages. The new sanity checks on MsgVoteResp
(m.Term != 0) would panic with the old test as raft.Term would be equal
to 0 when responding with MsgVoteResp messages.
This test verifies that adding a node does not cause the leader to step
down until at least one full ElectionTick cycle elapses.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
I found that enabling the CheckQuorum flag led to spurious leader
elections when new nodes joined. It looks like in the time between a new
node joining the cluster, and that node first communicating with the
leader, the quorum check could fail because the new node looks inactive.
To solve this, set the RecentActive flag when nodes are first added.
This gives a grace period for the node to communicate before it causes
the quorum check to fail.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
Accumulation of old entries in the underlying array backing the
entries slice has been found to cause massive memory growth in
CockroachDB for workloads that do large (1MB) writes
(https://github.com/cockroachdb/cockroach/issues/14776)
This doesn't appear to have much consistent effect on the raft
benchmarks, although it's worth noting that they vary quite a bit
between runs so it's kind of tough to draw strong conclusions from them.
Let me know if there are any different benchmarks you'd like me to run!
Fixes#7746
benchmark old ns/op new ns/op delta
BenchmarkOneNode-8 3283 3125 -4.81%
benchmark old allocs new allocs delta
BenchmarkOneNode-8 6 6 +0.00%
benchmark old bytes new bytes delta
BenchmarkOneNode-8 796 727 -8.67%
benchmark old ns/op new ns/op delta
BenchmarkProposal3Nodes-8 4269 4337 +1.59%
benchmark old allocs new allocs delta
BenchmarkProposal3Nodes-8 15 13 -13.33%
benchmark old bytes new bytes delta
BenchmarkProposal3Nodes-8 5839 4544 -22.18%
advance() should use rs.req.Entries[0].Data as the context instead of
req.Context for deletion. Since req.Context is never set, there won't be
any context being deleted from pendingReadIndex; results mem leak.
FIXES#7571
TestNodeTick relies on a unreliable func `waitForSchedule` when running
with GOMAXPROCS > 1. This commit changes the test to make sure we stop
the node afte it drains the tick chan. The test should be reliable now.
Getting gosimple suggestion while running test script, so this PR is for fixing gosimple S1019 check.
raft/node_test.go:456:40: should use make([]raftpb.Entry, 1) instead (S1019)
raft/node_test.go:457:49: should use make([]raftpb.Entry, 1) instead (S1019)
raft/node_test.go:458:43: should use make([]raftpb.Message, 1) instead (S1019)
Refer https://github.com/dominikh/go-tools/blob/master/cmd/gosimple/README.md#checks for more information.
n.Tick() is async. It can be racy when running with n.Stop().
n.Status() is sync and has a feedback mechnism internally. So there wont be
any race between n.Status() and n.Stop() call.
