From 29bebec0bb224893a163cb5df9336c37d41c350f Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Tue, 11 Apr 2017 09:39:49 -0400 Subject: [PATCH] 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 Signed-off-by: Robert Varga --- .../cluster/raft/FollowerLogInformation.java | 4 ++-- .../raft/FollowerLogInformationImpl.java | 9 +++++++-- .../cluster/raft/behaviors/AbstractLeader.java | 11 ++++++----- .../raft/FollowerLogInformationImplTest.java | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+), 9 deletions(-) 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 d40589ad2b..123d206d05 100644 --- 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 c899967306..883bfbb4e4 100644 --- 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 74e8b2049f..4382cfec32 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 @@ -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 3818b30f84..d68339ee86 100644 --- 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()); + } } -- 2.36.6