Bug 8206: Prevent decr follower next index beyong -1

If a follower's next index is already -1, we shouldn't decrement it
further, ie -1 is the lowest allowed value. This can result in AbstractLeader
continuously decrementing and logging an info message while in the
process of sending an install snapshot.

member-3-shard-default-config (Leader): follower member-1-shard-default-config last log term 2 conflicts with the leader's 3 - dec next index to -2

Modified decrNextIndex to return a boolean if next index was decremented
which is checked  by AbstractLeader.

Change-Id: I29454d4e71a7f9128b3b47f6a4e3403615c2c8d2
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
index d40589ad2bc26c7444306dd25e20c7e8f9c65a54..123d206d05b9e20742a3c45aba6b2f5bf692d09d 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
@@ -27,9 +27,9 @@ public interface FollowerLogInformation {
     /**
      * Decrements the value of the follower's next index.
      *
-     * @return the new value of nextIndex,
+     * @return true if the next index was decremented, ie it was previously >= 0, false otherwise.
      */
-    long decrNextIndex();
+    boolean decrNextIndex();
 
     /**
      * Sets the index of the follower's next log entry.

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
index c89996730694844cb2e35bb7ab333898e47e4345..883bfbb4e4184f0cbf0843002dfa3498ca13f622 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.java
@@ -66,8 +66,13 @@ public class FollowerLogInformationImpl implements FollowerLogInformation {
     }
 
     @Override
-    public long decrNextIndex() {
-        return nextIndex--;
+    public boolean decrNextIndex() {
+        if (nextIndex >= 0) {
+            nextIndex--;
+            return true;
+        }
+
+        return false;
     }
 
     @Override

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 74e8b2049f1506d148c4c4782c17091266209778..4382cfec3237075087837b5f1402cadab249188f 100644 (file)
--- 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
@@ -288,12 +288,13 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
                 // 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.
 
-                followerLogInformation.decrNextIndex();
-                updated = true;
+                if (followerLogInformation.decrNextIndex()) {
+                    updated = true;
 
-                log.info("{}: follower {} last log term {} conflicts with the leader's {} - dec next index to {}",
-                        logName(), followerId, appendEntriesReply.getLogLastTerm(), followersLastLogTermInLeadersLog,
-                        followerLogInformation.getNextIndex());
+                    log.info("{}: follower {} last log term {} conflicts with the leader's {} - dec next index to {}",
+                            logName(), followerId, appendEntriesReply.getLogLastTerm(),
+                            followersLastLogTermInLeadersLog, followerLogInformation.getNextIndex());
+                }
             }
         }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImplTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImplTest.java
index 3818b30f84643181d2bd7b03faabf83371f40e19..d68339ee8664f445b02a54cc46fb705bb3bf559e 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImplTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImplTest.java
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.controller.cluster.raft;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -112,4 +113,21 @@ public class FollowerLogInformationImplTest {
         followerLogInformation.markFollowerActive();
         assertTrue(followerLogInformation.isFollowerActive());
     }
+
+    @Test
+    public void testDecrNextIndex() {
+        MockRaftActorContext context = new MockRaftActorContext();
+        context.setCommitIndex(1);
+        FollowerLogInformation followerLogInformation =
+                new FollowerLogInformationImpl(new PeerInfo("follower1", null, VotingState.VOTING), 1, context);
+
+        assertTrue(followerLogInformation.decrNextIndex());
+        assertEquals("getNextIndex", 0, followerLogInformation.getNextIndex());
+
+        assertTrue(followerLogInformation.decrNextIndex());
+        assertEquals("getNextIndex", -1, followerLogInformation.getNextIndex());
+
+        assertFalse(followerLogInformation.decrNextIndex());
+        assertEquals("getNextIndex", -1, followerLogInformation.getNextIndex());
+    }
 }