Avoid unnecessary unsuccessful AppendEntriesReply 05/78805/4
authorTom Pantelis <tompantelis@gmail.com>
Fri, 14 Dec 2018 17:36:16 +0000 (12:36 -0500)
committerStephen Kitt <skitt@redhat.com>
Tue, 18 Dec 2018 17:09:53 +0000 (17:09 +0000)
commitc77c163d54872612b47f0f2550931fa408ed596b
treefef6de9ab1bf1e48f743921f14291fc71a1ff2ee
parent9905bf0575ff196a531eb114e89b1bdb7226bc6c
Avoid unnecessary unsuccessful AppendEntriesReply

In some CSIT runs I see messages such as "The prevLogIndex 10 was
found in the log but the term -1 is not equal to the append entries
prevLogTerm 2" which results in the Follower sending an unsuccessful
reply indicating it's out-of-sync. This causes the Leader to try to
catch it up.

However in the test cases, the Follower isn't actually
out-of-sync but rather it gets behind enough (the test issues 1000
txns per second per node) such that the leader re-sends AppendEntries
with log entries it already sent. When the Follower eventually receives
the first AppendEntries, it appends the entries and may do a "fake"
snapshot, ie trim the in-memory journal list and advance the snapshot
index. If so, when the duplicate AppendEntries is received, the leader's
previous log index is no longer in the Follower's journal log.
In Follower.isOutOfSync, isLogEntryPresent returns true b/c the leader's
previous index is included in the last snapshot even though it was trimmed
from the journal log. However, getLogEntryTerm returns -1 b/c it
checks if the index is in the journal log or equal to the snapshot
index, ie it doesn't take into account that the index may be in the
last snapshot. This is inconsistent with the first check and results in
the Follower reporting that it's out-of-sync when it really isnt.

I've also seen a negative result of this on the Leader side when a Follower
reports it's out-of-sync - the Follower's last log index is in the snapshot
but getLogEntryTerm returns -1 causing the Leader to take the last resort,
inefficient path of decrementing the Follower's next index in order to
catch it up.

Therefore I changed these 2 cases to also check if the index is in the snapshot
and, if so, use the snapshot term.

Change-Id: I4331d8fee85789a7004a692abb1b9c629eecd570
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/AbstractReplicatedLogImpl.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java