From: Tom Pantelis <tompantelis@gmail.com>
Date: Tue, 29 Aug 2017 19:42:12 +0000 (-0400)
Subject: Fix intermittent testOwnerChangesOnPeerAvailabilityChanges failure
X-Git-Tag: release/oxygen~124
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=1b0f84c4957e464bad6f7cb7350a8171c3d1621b

Fix intermittent testOwnerChangesOnPeerAvailabilityChanges failure

EntityOwnershipShardTest.testOwnerChangesOnPeerAvailabilityChanges:647->AbstractEntityOwnershipTest.verifyRaftState:280->lambda$testOwnerChangesOnPeerAvailabilityChanges$2:648 getRaftState expected:<[]Leader> but was:<[Pre]Leader>

It seems this was indirectly introduced by the addition of the
PurgeTransactionPayload - changes the timing of things a bit. I added
code to ensure peer2's lastAppliedIndex is up-to-date with the leader's
prior to stopping the leader to make it deterministic (ie peer2 should
be able to go straight to Leader).

Change-Id: I9abb950c7dc67b2d481d07b9b421ae46421b6510
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
---

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
index 73112818f2..a40e75f4c3 100644
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
@@ -301,8 +301,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
 
                 log.info("{}: follower {} appears to be behind the leader from the last snapshot - "
                     + "updated: matchIndex: {}, nextIndex: {}", logName(), followerId,
-                    appendEntriesReply.getLogLastTerm(), followerLogInformation.getMatchIndex(),
-                    followerLogInformation.getNextIndex());
+                    followerLogInformation.getMatchIndex(), followerLogInformation.getNextIndex());
             } else {
                 // The follower's log conflicts with leader's log so decrement follower's next index by 1
                 // in an attempt to find where the logs match.
diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/PreLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/PreLeader.java
index bb54688437..6d2f429de2 100644
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/PreLeader.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/PreLeader.java
@@ -40,6 +40,8 @@ public class PreLeader extends AbstractLeader {
     @Override
     public RaftActorBehavior handleMessage(ActorRef sender, Object message) {
         if (message instanceof ApplyState) {
+            log.debug("{}: Received {} - lastApplied: {}, lastIndex: {}", logName(), message, context.getLastApplied(),
+                    context.getReplicatedLog().lastIndex());
             if (context.getLastApplied() >= context.getReplicatedLog().lastIndex()) {
                 // We've applied all entries - we can switch to Leader.
                 return internalSwitchBehavior(new Leader(context, this));
diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java
index cfef02c509..276f11f936 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java
@@ -36,6 +36,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Predicate;
 import org.junit.After;
 import org.junit.Test;
@@ -630,6 +631,16 @@ public class EntityOwnershipShardTest extends AbstractEntityOwnershipTest {
         verifyOwner(peer1, ENTITY_TYPE, ENTITY_ID3, LOCAL_MEMBER_NAME);
         verifyOwner(peer1, ENTITY_TYPE, ENTITY_ID4, "");
 
+        AtomicLong leaderLastApplied = new AtomicLong();
+        verifyRaftState(leader, rs -> {
+            assertEquals("LastApplied up-to-date", rs.getLastApplied(), rs.getLastIndex());
+            leaderLastApplied.set(rs.getLastApplied());
+        });
+
+        verifyRaftState(peer2, rs -> {
+            assertEquals("LastApplied", leaderLastApplied.get(), rs.getLastIndex());
+        });
+
         // Kill the local leader and elect peer2 the leader. This should cause a new owner to be selected for
         // the entities (1 and 3) previously owned by the local leader member.