Bug 8206: Prevent decr follower next index beyong -1 80/54780/6
authorTom Pantelis <tompantelis@gmail.com>
Tue, 11 Apr 2017 13:39:49 +0000 (09:39 -0400)
committerTom Pantelis <tompantelis@gmail.com>
Fri, 14 Apr 2017 17:12:48 +0000 (17:12 +0000)
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>
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformation.java
opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImpl.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/test/java/org/opendaylight/controller/cluster/raft/FollowerLogInformationImplTest.java

index d40589a..123d206 100644 (file)
@@ -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 &gt;= 0, false otherwise.
      */
-    long decrNextIndex();
+    boolean decrNextIndex();
 
     /**
      * Sets the index of the follower's next log entry.
index c899967..883bfbb 100644 (file)
@@ -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
index 74e8b20..4382cfe 100644 (file)
@@ -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());
+                }
             }
         }
 
index 3818b30..d68339e 100644 (file)
@@ -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());
+    }
 }

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.