From a785966182ba80592f99be5a27a9af1d4c2ac37f Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Fri, 14 Aug 2015 08:42:02 -0400 Subject: [PATCH] Bug 4105: Implement UnregisterCandidateLocal in EntityOwnershipShard Also added a testOwnershipChanges case to EntityOwnershipShardTest to run thru various ownership change scenarios with local and remote candidates and local unregistration. As a result I found a couple bugs that I fixed. Change-Id: I4343754fbbc8f471975e6c723ffc0beaedee2860 Signed-off-by: Tom Pantelis --- ...dEntityOwnershipCandidateRegistration.java | 2 +- .../DistributedEntityOwnershipService.java | 5 +- .../EntityOwnerChangeListener.java | 26 +-- .../entityownership/EntityOwnershipShard.java | 23 ++- .../messages/UnregisterCandidateLocal.java | 13 +- .../AbstractEntityOwnershipTest.java | 48 ++++- ...DistributedEntityOwnershipServiceTest.java | 1 + .../EntityOwnerChangeListenerTest.java | 14 +- .../EntityOwnershipShardTest.java | 169 +++++++++++++++--- 9 files changed, 243 insertions(+), 58 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipCandidateRegistration.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipCandidateRegistration.java index 70c454e3af..94c7aa0976 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipCandidateRegistration.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipCandidateRegistration.java @@ -27,6 +27,6 @@ class DistributedEntityOwnershipCandidateRegistration extends AbstractEntityOwne @Override public void close() { - service.unregisterCandidate(getEntity()); + service.unregisterCandidate(getEntity(), getInstance()); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipService.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipService.java index f51f579443..bfdda4ce70 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipService.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipService.java @@ -76,7 +76,6 @@ public class DistributedEntityOwnershipService implements EntityOwnershipService public void onComplete(Throwable failure, Object response) { if(failure != null) { LOG.debug("Error sending message {} to {}", message, shardActor, failure); - // TODO - queue for retry } else { LOG.debug("{} message to {} succeeded", message, shardActor, failure); } @@ -121,10 +120,10 @@ public class DistributedEntityOwnershipService implements EntityOwnershipService return new DistributedEntityOwnershipCandidateRegistration(candidate, entity, this); } - void unregisterCandidate(Entity entity) { + void unregisterCandidate(Entity entity, EntityOwnershipCandidate entityOwnershipCandidate) { LOG.debug("Unregistering candidate for {}", entity); - executeLocalEntityOwnershipShardOperation(new UnregisterCandidateLocal(entity)); + executeLocalEntityOwnershipShardOperation(new UnregisterCandidateLocal(entityOwnershipCandidate, entity)); registeredEntities.remove(entity); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListener.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListener.java index 253761fcb4..9c551f2576 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListener.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListener.java @@ -28,6 +28,7 @@ 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.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -55,30 +56,33 @@ class EntityOwnerChangeListener implements DOMDataTreeChangeListener { @Override public void onDataTreeChanged(Collection changes) { for(DataTreeCandidate change: changes) { - MapEntryNode entityNode = (MapEntryNode) change.getRootNode().getDataAfter().get(); + DataTreeCandidateNode changeRoot = change.getRootNode(); + MapEntryNode entityNode = (MapEntryNode) changeRoot.getDataAfter().get(); - LOG.debug("Entity node updated: {}", change.getRootPath()); + LOG.debug("Entity node changed: {}, {}", changeRoot.getModificationType(), change.getRootPath()); String newOwner = extractOwner(entityNode); String origOwner = null; - Optional> dataBefore = change.getRootNode().getDataBefore(); + Optional> dataBefore = changeRoot.getDataBefore(); if(dataBefore.isPresent()) { - MapEntryNode origEntityNode = (MapEntryNode) change.getRootNode().getDataBefore().get(); + MapEntryNode origEntityNode = (MapEntryNode) changeRoot.getDataBefore().get(); origOwner = extractOwner(origEntityNode); } LOG.debug("New owner: {}, Original owner: {}", newOwner, origOwner); - boolean isOwner = Objects.equal(localMemberName, newOwner); - boolean wasOwner = Objects.equal(localMemberName, origOwner); - if(isOwner || wasOwner) { - Entity entity = createEntity(change.getRootPath()); + if(!Objects.equal(origOwner, newOwner)) { + boolean isOwner = Objects.equal(localMemberName, newOwner); + boolean wasOwner = Objects.equal(localMemberName, origOwner); + if(isOwner || wasOwner) { + Entity entity = createEntity(change.getRootPath()); - LOG.debug("Calling notifyEntityOwnershipListeners: entity: {}, wasOwner: {}, isOwner: {}", - entity, wasOwner, isOwner); + LOG.debug("Calling notifyEntityOwnershipListeners: entity: {}, wasOwner: {}, isOwner: {}", + entity, wasOwner, isOwner); - listenerSupport.notifyEntityOwnershipListeners(entity, wasOwner, isOwner); + listenerSupport.notifyEntityOwnershipListeners(entity, wasOwner, isOwner); + } } } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java index 175e854269..b1e63ae4d9 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java @@ -10,12 +10,14 @@ package org.opendaylight.controller.cluster.datastore.entityownership; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNERS_PATH; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNER_NODE_ID; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNER_QNAME; +import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.candidatePath; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.entityOwnersWithCandidate; import akka.actor.ActorRef; import akka.actor.ActorSelection; import akka.actor.Props; import akka.pattern.Patterns; import com.google.common.base.Optional; +import com.google.common.base.Strings; import java.util.Collection; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -28,8 +30,10 @@ import org.opendaylight.controller.cluster.datastore.entityownership.messages.Un import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; import org.opendaylight.controller.cluster.datastore.messages.BatchedModifications; import org.opendaylight.controller.cluster.datastore.messages.SuccessReply; +import org.opendaylight.controller.cluster.datastore.modification.DeleteModification; import org.opendaylight.controller.cluster.datastore.modification.MergeModification; 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.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; @@ -99,6 +103,18 @@ class EntityOwnershipShard extends Shard { getSender().tell(SuccessReply.INSTANCE, getSelf()); } + private void onUnregisterCandidateLocal(UnregisterCandidateLocal unregisterCandidate) { + LOG.debug("onUnregisterCandidateLocal: {}", unregisterCandidate); + + Entity entity = unregisterCandidate.getEntity(); + listenerSupport.removeEntityOwnershipListener(entity, unregisterCandidate.getCandidate()); + + YangInstanceIdentifier candidatePath = candidatePath(entity.getType(), entity.getId(), localMemberName); + commitCoordinator.commitModification(new DeleteModification(candidatePath), this); + + getSender().tell(SuccessReply.INSTANCE, getSelf()); + } + void tryCommitModifications(final BatchedModifications modifications) { if(isLeader()) { LOG.debug("Committing BatchedModifications {} locally", modifications.getTransactionID()); @@ -132,11 +148,6 @@ class EntityOwnershipShard extends Shard { commitCoordinator.onStateChanged(this, isLeader()); } - private void onUnregisterCandidateLocal(UnregisterCandidateLocal unregisterCandidate) { - // TODO - implement - getSender().tell(SuccessReply.INSTANCE, getSelf()); - } - private void onCandidateRemoved(CandidateRemoved message) { if(!isLeader()){ return; @@ -158,7 +169,7 @@ class EntityOwnershipShard extends Shard { LOG.debug("onCandidateAdded: {}", message); String currentOwner = getCurrentOwner(message.getEntityPath()); - if(currentOwner == null){ + if(Strings.isNullOrEmpty(currentOwner)){ writeNewOwner(message.getEntityPath(), newOwner(message.getAllCandidates())); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/messages/UnregisterCandidateLocal.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/messages/UnregisterCandidateLocal.java index d99ec7b154..04d7960b7f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/messages/UnregisterCandidateLocal.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/messages/UnregisterCandidateLocal.java @@ -8,6 +8,7 @@ package org.opendaylight.controller.cluster.datastore.entityownership.messages; import org.opendaylight.controller.md.sal.common.api.clustering.Entity; +import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipCandidate; /** * Message sent to the local EntityOwnershipShard to unregister a candidate. @@ -15,20 +16,24 @@ import org.opendaylight.controller.md.sal.common.api.clustering.Entity; * @author Thomas Pantelis */ public class UnregisterCandidateLocal { + private final EntityOwnershipCandidate candidate; private final Entity entity; - public UnregisterCandidateLocal(Entity entity) { + public UnregisterCandidateLocal(EntityOwnershipCandidate candidate, Entity entity) { + this.candidate = candidate; this.entity = entity; } + public EntityOwnershipCandidate getCandidate() { + return candidate; + } + public Entity getEntity() { return entity; } @Override public String toString() { - StringBuilder builder = new StringBuilder(); - builder.append("UnregisterCandidateLocal [entity=").append(entity).append("]"); - return builder.toString(); + return "UnregisterCandidateLocal [entity=" + entity + ", candidate=" + candidate + "]"; } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java index 0e282fbbbf..525e03bb37 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnershipTest.java @@ -13,6 +13,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.CANDIDATE_NAME_QNAME; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_ID_QNAME; +import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNERS_PATH; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNER_QNAME; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_QNAME; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_TYPE_QNAME; @@ -68,6 +69,24 @@ public class AbstractEntityOwnershipTest extends AbstractActorTest { } } + protected void verifyEntityCandidate(String entityType, YangInstanceIdentifier entityId, String candidateName, + Function> reader) { + AssertionError lastError = null; + Stopwatch sw = Stopwatch.createStarted(); + while(sw.elapsed(TimeUnit.MILLISECONDS) <= 5000) { + NormalizedNode node = reader.apply(ENTITY_OWNERS_PATH); + try { + verifyEntityCandidate(node, entityType, entityId, candidateName); + return; + } catch (AssertionError e) { + lastError = e; + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); + } + } + + throw lastError; + } + protected MapEntryNode getMapEntryNodeChild(DataContainerNode parent, QName childMap, QName child, Object key) { Optional> childNode = @@ -85,19 +104,40 @@ public class AbstractEntityOwnershipTest extends AbstractActorTest { protected void verifyOwner(String expected, String entityType, YangInstanceIdentifier entityId, Function> reader) { + AssertionError lastError = null; YangInstanceIdentifier entityPath = entityPath(entityType, entityId).node(ENTITY_OWNER_QNAME); Stopwatch sw = Stopwatch.createStarted(); while(sw.elapsed(TimeUnit.MILLISECONDS) <= 5000) { - NormalizedNode node = reader.apply(entityPath); - if(node != null) { + try { + NormalizedNode node = reader.apply(entityPath); + Assert.assertNotNull("Owner was not set for entityId: " + entityId, node); Assert.assertEquals("Entity owner", expected, node.getValue().toString()); return; - } else { + } catch(AssertionError e) { + lastError = e; + Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); + } + } + + throw lastError; + } + + protected void verifyNodeRemoved(YangInstanceIdentifier path, + Function> reader) { + AssertionError lastError = null; + Stopwatch sw = Stopwatch.createStarted(); + while(sw.elapsed(TimeUnit.MILLISECONDS) <= 5000) { + try { + NormalizedNode node = reader.apply(path); + Assert.assertNull("Node was not removed at path: " + path, node); + return; + } catch(AssertionError e) { + lastError = e; Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); } } - fail("Owner was not set for entityId: " + entityId); + throw lastError; } static void writeNode(YangInstanceIdentifier path, NormalizedNode node, ShardDataTree shardDataTree) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipServiceTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipServiceTest.java index 7e789ae261..0776b502c2 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipServiceTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipServiceTest.java @@ -196,6 +196,7 @@ public class DistributedEntityOwnershipServiceTest extends AbstractEntityOwnersh UnregisterCandidateLocal unregCandidate = shardPropsCreator.waitForShardMessage(); assertEquals("getEntity", entity, unregCandidate.getEntity()); + assertSame("getCandidate", candidate, unregCandidate.getCandidate()); // Re-register - should succeed. diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListenerTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListenerTest.java index e87b406f39..9bd0ccf2c0 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListenerTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnerChangeListenerTest.java @@ -55,39 +55,35 @@ public class EntityOwnerChangeListenerTest { } @Test - public void testOnDataChanged() throws Exception { + public void testOnDataTreeChanged() throws Exception { writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME)); writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID2, LOCAL_MEMBER_NAME)); - writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, REMOTE_MEMBER_NAME1)); - verify(mockListenerSupport, never()).notifyEntityOwnershipListeners(any(Entity.class), anyBoolean(), anyBoolean()); writeNode(entityPath(ENTITY_TYPE, ENTITY_ID1), entityEntryWithOwner(ENTITY_ID1, LOCAL_MEMBER_NAME)); - verify(mockListenerSupport).notifyEntityOwnershipListeners(ENTITY1, false, true); reset(mockListenerSupport); - writeNode(entityPath(ENTITY_TYPE, ENTITY_ID1), entityEntryWithOwner(ENTITY_ID1, REMOTE_MEMBER_NAME1)); + writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, REMOTE_MEMBER_NAME1)); + verify(mockListenerSupport, never()).notifyEntityOwnershipListeners(any(Entity.class), anyBoolean(), anyBoolean()); + reset(mockListenerSupport); + writeNode(entityPath(ENTITY_TYPE, ENTITY_ID1), entityEntryWithOwner(ENTITY_ID1, REMOTE_MEMBER_NAME1)); verify(mockListenerSupport).notifyEntityOwnershipListeners(ENTITY1, true, false); reset(mockListenerSupport); writeNode(entityPath(ENTITY_TYPE, ENTITY_ID1), entityEntryWithOwner(ENTITY_ID1, REMOTE_MEMBER_NAME2)); - verify(mockListenerSupport, never()).notifyEntityOwnershipListeners(any(Entity.class), anyBoolean(), anyBoolean()); writeNode(entityPath(ENTITY_TYPE, ENTITY_ID1), entityEntryWithOwner(ENTITY_ID1, LOCAL_MEMBER_NAME)); - verify(mockListenerSupport).notifyEntityOwnershipListeners(ENTITY1, false, true); reset(mockListenerSupport); writeNode(entityPath(ENTITY_TYPE, ENTITY_ID2), entityEntryWithOwner(ENTITY_ID2, REMOTE_MEMBER_NAME1)); - verify(mockListenerSupport, never()).notifyEntityOwnershipListeners(any(Entity.class), anyBoolean(), anyBoolean()); reset(mockListenerSupport); writeNode(entityPath(ENTITY_TYPE, ENTITY_ID2), entityEntryWithOwner(ENTITY_ID2, LOCAL_MEMBER_NAME)); - verify(mockListenerSupport).notifyEntityOwnershipListeners(ENTITY2, false, true); } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java index 49953ac143..ec2006d683 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardTest.java @@ -8,17 +8,22 @@ package org.opendaylight.controller.cluster.datastore.entityownership; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.ENTITY_OWNERS_PATH; +import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.candidatePath; +import static org.opendaylight.controller.cluster.datastore.entityownership.EntityOwnersModel.entityOwnersWithCandidate; import akka.actor.ActorRef; import akka.actor.Props; import akka.actor.UntypedActor; import akka.dispatch.Dispatchers; import akka.testkit.TestActorRef; import com.google.common.base.Function; -import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Uninterruptibles; import java.util.ArrayList; @@ -34,8 +39,10 @@ import org.opendaylight.controller.cluster.datastore.AbstractShardTest; import org.opendaylight.controller.cluster.datastore.DataStoreVersions; import org.opendaylight.controller.cluster.datastore.DatastoreContext; import org.opendaylight.controller.cluster.datastore.DatastoreContext.Builder; +import org.opendaylight.controller.cluster.datastore.ShardDataTree; import org.opendaylight.controller.cluster.datastore.ShardTestKit; import org.opendaylight.controller.cluster.datastore.entityownership.messages.RegisterCandidateLocal; +import org.opendaylight.controller.cluster.datastore.entityownership.messages.UnregisterCandidateLocal; import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; import org.opendaylight.controller.cluster.datastore.messages.BatchedModifications; import org.opendaylight.controller.cluster.datastore.messages.CommitTransactionReply; @@ -98,9 +105,7 @@ public class EntityOwnershipShardTest extends AbstractEntityOwnershipTest { kit.expectMsgClass(SuccessReply.class); verifyCommittedEntityCandidate(shard, ENTITY_TYPE, entityId, LOCAL_MEMBER_NAME); - verifyOwner(shard, ENTITY_TYPE, entityId, LOCAL_MEMBER_NAME); - verify(candidate, timeout(5000)).ownershipChanged(entity, false, true); } @@ -292,9 +297,147 @@ public class EntityOwnershipShardTest extends AbstractEntityOwnershipTest { assertEquals("# modifications received", max, receivedMods.size()); } - private void verifyCommittedEntityCandidate(TestActorRef shard, String entityType, - YangInstanceIdentifier entityId, String candidateName) throws Exception { - verifyEntityCandidate(readEntityOwners(shard), entityType, entityId, candidateName); + @Test + public void testOnUnregisterCandidateLocal() throws Exception { + ShardTestKit kit = new ShardTestKit(getSystem()); + TestActorRef shard = actorFactory.createTestActor(newShardProps()); + kit.waitUntilLeader(shard); + + Entity entity = new Entity(ENTITY_TYPE, ENTITY_ID1); + EntityOwnershipCandidate candidate = mock(EntityOwnershipCandidate.class); + + // Register + + shard.tell(new RegisterCandidateLocal(candidate, entity), kit.getRef()); + kit.expectMsgClass(SuccessReply.class); + + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verify(candidate, timeout(5000)).ownershipChanged(entity, false, true); + + // Unregister + + reset(candidate); + + shard.tell(new UnregisterCandidateLocal(candidate, entity), kit.getRef()); + kit.expectMsgClass(SuccessReply.class); + + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, ""); + verify(candidate, never()).ownershipChanged(any(Entity.class), anyBoolean(), anyBoolean()); + + // Register again + + shard.tell(new RegisterCandidateLocal(candidate, entity), kit.getRef()); + kit.expectMsgClass(SuccessReply.class); + + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verify(candidate, timeout(5000)).ownershipChanged(entity, false, true); + } + + @Test + public void testOwnershipChanges() throws Exception { + ShardTestKit kit = new ShardTestKit(getSystem()); + TestActorRef shard = actorFactory.createTestActor(newShardProps()); + kit.waitUntilLeader(shard); + + Entity entity = new Entity(ENTITY_TYPE, ENTITY_ID1); + EntityOwnershipCandidate candidate = mock(EntityOwnershipCandidate.class); + ShardDataTree shardDataTree = shard.underlyingActor().getDataStore(); + + // Add a remote candidate + + String remoteMemberName1 = "remoteMember1"; + writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, remoteMemberName1), shardDataTree); + + // Register local + + shard.tell(new RegisterCandidateLocal(candidate, entity), kit.getRef()); + kit.expectMsgClass(SuccessReply.class); + + // Verify the remote candidate becomes owner + + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName1); + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName1); + verify(candidate, never()).ownershipChanged(any(Entity.class), anyBoolean(), anyBoolean()); + + // Add another remote candidate and verify ownership doesn't change + + reset(candidate); + String remoteMemberName2 = "remoteMember2"; + writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, remoteMemberName2), shardDataTree); + + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName2); + Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName1); + verify(candidate, never()).ownershipChanged(any(Entity.class), anyBoolean(), anyBoolean()); + + // Remove the second remote candidate and verify ownership doesn't change + + reset(candidate); + deleteNode(candidatePath(ENTITY_TYPE, ENTITY_ID1, remoteMemberName2), shardDataTree); + + verifyEntityCandidateRemoved(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName2); + Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName1); + verify(candidate, never()).ownershipChanged(any(Entity.class), anyBoolean(), anyBoolean()); + + // Remove the first remote candidate and verify the local candidate becomes owner + + reset(candidate); + deleteNode(candidatePath(ENTITY_TYPE, ENTITY_ID1, remoteMemberName1), shardDataTree); + + verifyEntityCandidateRemoved(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName1); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verify(candidate, timeout(5000)).ownershipChanged(entity, false, true); + + // Add the second remote candidate back and verify ownership doesn't change + + reset(candidate); + writeNode(ENTITY_OWNERS_PATH, entityOwnersWithCandidate(ENTITY_TYPE, ENTITY_ID1, remoteMemberName2), shardDataTree); + + verifyCommittedEntityCandidate(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName2); + Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verify(candidate, never()).ownershipChanged(any(Entity.class), anyBoolean(), anyBoolean()); + + // Unregister the local candidate and verify the second remote candidate becomes owner + + shard.tell(new UnregisterCandidateLocal(candidate, entity), kit.getRef()); + kit.expectMsgClass(SuccessReply.class); + + verifyEntityCandidateRemoved(shard, ENTITY_TYPE, ENTITY_ID1, LOCAL_MEMBER_NAME); + verifyOwner(shard, ENTITY_TYPE, ENTITY_ID1, remoteMemberName2); + } + + private void verifyEntityCandidateRemoved(final TestActorRef shard, String entityType, + YangInstanceIdentifier entityId, String candidateName) { + verifyNodeRemoved(candidatePath(entityType, entityId, candidateName), + new Function>() { + @Override + public NormalizedNode apply(YangInstanceIdentifier path) { + try { + return AbstractShardTest.readStore(shard, path); + } catch(Exception e) { + throw new AssertionError("Failed to read " + path, e); + } + } + }); + } + + private void verifyCommittedEntityCandidate(final TestActorRef shard, String entityType, + YangInstanceIdentifier entityId, String candidateName) { + verifyEntityCandidate(entityType, entityId, candidateName, new Function>() { + @Override + public NormalizedNode apply(YangInstanceIdentifier path) { + try { + return AbstractShardTest.readStore(shard, path); + } catch(Exception e) { + throw new AssertionError("Failed to read " + path, e); + } + } + }); } private void verifyBatchedEntityCandidate(List mods, String entityType, @@ -310,20 +453,6 @@ public class EntityOwnershipShardTest extends AbstractEntityOwnershipTest { entityId, candidateName); } - private NormalizedNode readEntityOwners(TestActorRef shard) throws Exception { - Stopwatch sw = Stopwatch.createStarted(); - while(sw.elapsed(TimeUnit.MILLISECONDS) <= 5000) { - NormalizedNode node = AbstractShardTest.readStore(shard, ENTITY_OWNERS_PATH); - if(node != null) { - return node; - } - - Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); - } - - return null; - } - private void verifyOwner(final TestActorRef shard, String entityType, YangInstanceIdentifier entityId, String localMemberName) { verifyOwner(localMemberName, entityType, entityId, new Function>() { -- 2.36.6