BUG 3054 : Check readiness status of datastore when receiving MemberUp. 50/19250/3
authorMoiz Raja <moraja@cisco.com>
Tue, 28 Apr 2015 18:15:51 +0000 (11:15 -0700)
committerTom Pantelis <tpanteli@brocade.com>
Sun, 26 Apr 2015 00:55:11 +0000 (20:55 -0400)
A Follower knows who the Leader is when it receives an AppendEntries message,
however it does record the address of the leader, that is supplied by the
ShardManager when the ShardManager receives a MemberUp. The Follower cannot
be considered ready till it knows both who the leader is and what is it's
address. This patch ensures that the ShardManager counts down the readiness
latch only when the follower knows both.

This also ensures that FindPrimaryShard will only return a primary if the Leader
is truly known.

Change-Id: I1cd6b62124d50c9eca37368d0379b0c75168bb76
Signed-off-by: Moiz Raja <moraja@cisco.com>
(cherry picked from commit bc5e040c7362243607e04045ac9d9309817bb9c1)

opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardManager.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardManagerTest.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/utils/MockClusterWrapper.java

index f32ba4e..f5baf69 100644 (file)
@@ -435,6 +435,8 @@ public class ShardManager extends AbstractUntypedPersistentActorWithMetering {
             info.updatePeerAddress(getShardIdentifier(memberName, shardName).toString(),
                 getShardActorPath(shardName, memberName), getSelf());
         }
+
+        checkReady();
     }
 
     private void onDatastoreContext(DatastoreContext context) {
@@ -727,7 +729,7 @@ public class ShardManager extends AbstractUntypedPersistentActorWithMetering {
         }
 
         boolean isShardReadyWithLeaderId() {
-            return isShardReady() && (isLeader() || peerAddresses.containsKey(leaderId));
+            return isShardReady() && (isLeader() || peerAddresses.get(leaderId) != null);
         }
 
         boolean isShardInitialized() {
index b676cf2..42f96f0 100644 (file)
@@ -168,6 +168,26 @@ public class ShardManagerTest extends AbstractActorTest {
         }};
     }
 
+    @Test
+    public void testOnReceiveFindPrimaryForNonLocalLeaderShardBeforeMemberUp() throws Exception {
+        new JavaTestKit(getSystem()) {{
+            final ActorRef shardManager = getSystem().actorOf(newPropsShardMgrWithMockShardActor());
+
+            shardManager.tell(new UpdateSchemaContext(TestModel.createTestContext()), getRef());
+            shardManager.tell(new ActorInitialized(), mockShardActor);
+
+            String memberId2 = "member-2-shard-default-" + shardMrgIDSuffix;
+            String memberId1 = "member-1-shard-default-" + shardMrgIDSuffix;
+            shardManager.tell(new RoleChangeNotification(memberId1,
+                    RaftState.Candidate.name(), RaftState.Follower.name()), mockShardActor);
+            shardManager.tell(new LeaderStateChanged(memberId1, memberId2), mockShardActor);
+
+            shardManager.tell(new FindPrimary(Shard.DEFAULT_NAME, false), getRef());
+
+            expectMsgClass(duration("5 seconds"), NoShardLeaderException.class);
+        }};
+    }
+
     @Test
     public void testOnReceiveFindPrimaryForNonLocalLeaderShard() throws Exception {
         new JavaTestKit(getSystem()) {{
@@ -669,6 +689,8 @@ public class ShardManagerTest extends AbstractActorTest {
 
                 verify(ready, never()).countDown();
 
+                shardManager.underlyingActor().onReceiveCommand(MockClusterWrapper.createMemberUp("member-2", getRef().path().toString()));
+
                 shardManager.underlyingActor().onReceiveCommand(new LeaderStateChanged(memberId, "member-2-shard-default-" + shardMrgIDSuffix));
 
                 verify(ready, times(1)).countDown();
@@ -676,6 +698,26 @@ public class ShardManagerTest extends AbstractActorTest {
             }};
     }
 
+    @Test
+    public void testReadyCountDownForMemberUpAfterLeaderStateChanged() throws Exception {
+        new JavaTestKit(getSystem()) {
+            {
+                TestActorRef<ShardManager> shardManager = TestActorRef.create(getSystem(), newShardMgrProps());
+
+                String memberId = "member-1-shard-default-" + shardMrgIDSuffix;
+                shardManager.underlyingActor().onReceiveCommand(new RoleChangeNotification(
+                        memberId, null, RaftState.Follower.name()));
+
+                verify(ready, never()).countDown();
+
+                shardManager.underlyingActor().onReceiveCommand(new LeaderStateChanged(memberId, "member-2-shard-default-" + shardMrgIDSuffix));
+
+                shardManager.underlyingActor().onReceiveCommand(MockClusterWrapper.createMemberUp("member-2", getRef().path().toString()));
+
+                verify(ready, times(1)).countDown();
+
+            }};
+    }
 
     @Test
     public void testRoleChangeNotificationDoNothingForUnknownShard() throws Exception {
index 810b270..5d44033 100644 (file)
@@ -74,7 +74,7 @@ public class MockClusterWrapper implements ClusterWrapper{
     }
 
 
-    private static ClusterEvent.MemberUp createMemberUp(String memberName, String address) {
+    public static ClusterEvent.MemberUp createMemberUp(String memberName, String address) {
         akka.cluster.UniqueAddress uniqueAddress = new UniqueAddress(
             AddressFromURIString.parse(address), 55);
 

©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.