BUG-5247: notify listeners for entities which are not owned 79/34179/19
authorRobert Varga <rovarga@cisco.com>
Fri, 5 Feb 2016 17:45:40 +0000 (18:45 +0100)
committerGerrit Code Review <gerrit@opendaylight.org>
Mon, 15 Feb 2016 13:25:38 +0000 (13:25 +0000)
Rather than broadcasting just the 'up' state, notify listeners about all
state we know of.

Change-Id: Iaae6db925a321aad420fa0ee8bdf8b56b5d2a29e
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java

index 3a031fc6a4313b6a0177e2c8c66287ec00912676..fb7e9e4668b5cf7edf97996cf3629bf96fe150c8 100644 (file)
@@ -57,7 +57,6 @@ import org.opendaylight.controller.cluster.datastore.modification.MergeModificat
 import org.opendaylight.controller.cluster.datastore.modification.WriteModification;
 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
 import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapNode;
@@ -189,16 +188,29 @@ class EntityOwnershipShard extends Shard {
 
         getSender().tell(SuccessReply.INSTANCE, getSelf());
 
-        searchForEntitiesOwnedBy(localMemberName, new EntityWalker() {
+        searchForEntities(new EntityWalker() {
             @Override
             public void onEntity(MapEntryNode entityTypeNode, MapEntryNode entityNode) {
-                Optional<DataContainerChild<? extends PathArgument, ?>> possibleType =
-                        entityTypeNode.getChild(ENTITY_TYPE_NODE_ID);
+                Optional<DataContainerChild<?, ?>> possibleType = entityTypeNode.getChild(ENTITY_TYPE_NODE_ID);
                 String entityType = possibleType.isPresent() ? possibleType.get().getValue().toString() : null;
                 if (registerListener.getEntityType().equals(entityType)) {
+                    final boolean hasOwner;
+                    final boolean isOwner;
+
+                    Optional<DataContainerChild<?, ?>> possibleOwner = entityNode.getChild(ENTITY_OWNER_NODE_ID);
+                    if (possibleOwner.isPresent()) {
+                        isOwner = localMemberName.equals(possibleOwner.get().getValue().toString());
+                        hasOwner = true;
+                    } else {
+                        isOwner = false;
+                        hasOwner = false;
+                    }
+
                     Entity entity = new Entity(entityType,
-                            (YangInstanceIdentifier) entityNode.getChild(ENTITY_ID_NODE_ID).get().getValue());
-                    listenerSupport.notifyEntityOwnershipListener(entity, false, true, true, registerListener.getListener());
+                        (YangInstanceIdentifier) entityNode.getChild(ENTITY_ID_NODE_ID).get().getValue());
+
+                    listenerSupport.notifyEntityOwnershipListener(entity, false, isOwner, hasOwner,
+                        registerListener.getListener());
                 }
             }
         });
@@ -399,26 +411,6 @@ class EntityOwnershipShard extends Shard {
         return ((MapNode)entity.getChild(CANDIDATE_NODE_ID).get()).getChild(candidateNodeKey(candidateName)).isPresent();
     }
 
-    private void searchForEntitiesOwnedBy(final String owner, final EntityWalker walker) {
-        Optional<NormalizedNode<?, ?>> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH);
-        if(!possibleEntityTypes.isPresent()) {
-            return;
-        }
-
-        LOG.debug("{}: Searching for entities owned by {}", persistenceId(), owner);
-
-        searchForEntities(new EntityWalker() {
-            @Override
-            public void onEntity(MapEntryNode entityTypeNode, MapEntryNode entityNode) {
-                Optional<DataContainerChild<? extends PathArgument, ?>> possibleOwner =
-                        entityNode.getChild(ENTITY_OWNER_NODE_ID);
-                if (possibleOwner.isPresent() && owner.equals(possibleOwner.get().getValue().toString())) {
-                    walker.onEntity(entityTypeNode, entityNode);
-                }
-            }
-        });
-    }
-
     private void searchForEntities(EntityWalker walker) {
         Optional<NormalizedNode<?, ?>> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH);
         if(!possibleEntityTypes.isPresent()) {
@@ -426,10 +418,10 @@ class EntityOwnershipShard extends Shard {
         }
 
         for(MapEntryNode entityType:  ((MapNode) possibleEntityTypes.get()).getValue()) {
-            Optional<DataContainerChild<? extends PathArgument, ?>> possibleEntities =
-                    entityType.getChild(ENTITY_NODE_ID);
+            Optional<DataContainerChild<?, ?>> possibleEntities = entityType.getChild(ENTITY_NODE_ID);
             if(!possibleEntities.isPresent()) {
-                continue; // shouldn't happen but handle anyway
+                // shouldn't happen but handle anyway
+                continue;
             }
 
             for(MapEntryNode entity:  ((MapNode) possibleEntities.get()).getValue()) {
index 0b4935734f474089eea1b553b1e27b082dba3349..744ad220e3e8da734c8c910244ce1b7adb4b12a3 100644 (file)
@@ -12,7 +12,6 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.AdditionalMatchers.or;
-import static org.mockito.Mockito.atMost;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.timeout;
@@ -196,6 +195,9 @@ public class DistributedEntityOwnershipIntegrationTest {
         // Register follower2 candidate for entity2 and verify it gets added but doesn't become owner
 
         follower2EntityOwnershipService.registerListener(ENTITY_TYPE1, follower2MockListener);
+        verify(follower2MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY2, false, false, true));
+        verify(follower2MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY1, false, false, true));
+
         follower2EntityOwnershipService.registerCandidate(ENTITY2);
         verifyCandidates(leaderDistributedDataStore, ENTITY2, "member-2", "member-3");
         verifyOwner(leaderDistributedDataStore, ENTITY2, "member-2");
@@ -212,7 +214,6 @@ public class DistributedEntityOwnershipIntegrationTest {
         // if the original ownership change with "member-2 is replicated to follower2 after the listener is
         // registered.
         Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
-        verify(follower2MockListener, atMost(1)).ownershipChanged(ownershipChange(ENTITY2, false, false, true));
         verify(follower2MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY2, false, true, true));
 
         // Register follower1 candidate for entity3 and verify it becomes owner
index be7a42fda5428f7a1b1707113cd66ff89ccceae9..fc9b5298e150eb6ff895c3eb263852a9eaaff051 100644 (file)
@@ -12,6 +12,7 @@ import static org.mockito.AdditionalMatchers.or;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
@@ -710,7 +711,7 @@ public class EntityOwnershipShardTest extends AbstractEntityOwnershipTest {
                 ownershipChange(entity3, false, true, true)));
         Uninterruptibles.sleepUninterruptibly(300, TimeUnit.MILLISECONDS);
         verify(listener, never()).ownershipChanged(ownershipChange(entity4));
-        verify(listener, never()).ownershipChanged(ownershipChange(entity1));
+        verify(listener, times(1)).ownershipChanged(ownershipChange(entity1));
     }
 
     private static void commitModification(TestActorRef<EntityOwnershipShard> shard, NormalizedNode<?, ?> node,