From acbbcc0278c08cf49d71a35b776608fee9e7d417 Mon Sep 17 00:00:00 2001 From: Moiz Raja Date: Fri, 8 Apr 2016 11:09:50 -0700 Subject: [PATCH] BUG 5690 : No owner present even when entity has a candidate If a candidate for an entity is removed and another added in quick succession it can leave the owner of the entity blank. This happens because the BatchedModifications for candidate removal happen one after another which results in the commit of those modifications. The BatchedModification which writes an owner on removal is committed only after the addition of the new candidate. In this scenario when the new candidate is added it finds that there is still an owner for that entity and so it does not assign a new owner for that entity. To fix this problem in onCandidateAdded we check if the currentOwner is present in the current candidate list and if it is not then we choose a new owner. Change-Id: I47f90314e018e25f2c1dac82342b931c4e2d882d Signed-off-by: Moiz Raja --- .../entityownership/EntityOwnershipShard.java | 5 +- ...ributedEntityOwnershipIntegrationTest.java | 86 ++++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) 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 0ca4980bbf..678546e167 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 @@ -347,8 +347,9 @@ class EntityOwnershipShard extends Shard { // So if there are 2 peers and 1 is down then availableMembers will be 2 final int availableMembers = (peerIdToMemberNames.size() - downPeerMemberNames.size()) + 1; - LOG.debug("{}: Using strategy {} to select owner", persistenceId(), strategy); - if(Strings.isNullOrEmpty(currentOwner)){ + LOG.debug("{}: Using strategy {} to select owner, currentOwner = {}", persistenceId(), strategy, currentOwner); + + if(!message.getAllCandidates().contains(currentOwner)){ if(strategy.getSelectionDelayInMillis() == 0L) { writeNewOwner(message.getEntityPath(), newOwner(currentOwner, message.getAllCandidates(), entityOwnershipStatistics.byEntityType(entityType), strategy)); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java index e049238e3b..78e2c91384 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java @@ -580,8 +580,90 @@ public class DistributedEntityOwnershipIntegrationTest { }); } - private static void verifyGetOwnershipState(final EntityOwnershipService service, final Entity entity, - final boolean isOwner, final boolean hasOwner) { + @Test + public void testOwnerSelectedOnRapidUnregisteringAndRegisteringOfCandidates() throws Exception { + String name = "test"; + MemberNode leaderNode = MemberNode.builder(memberNodes).akkaConfig("Member1").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(leaderDatastoreContextBuilder).build(); + + MemberNode follower1Node = MemberNode.builder(memberNodes).akkaConfig("Member2").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(followerDatastoreContextBuilder).build(); + + MemberNode follower2Node = MemberNode.builder(memberNodes).akkaConfig("Member3").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(followerDatastoreContextBuilder).build(); + + DistributedDataStore leaderDistributedDataStore = leaderNode.configDataStore(); + + leaderDistributedDataStore.waitTillReady(); + follower1Node.configDataStore().waitTillReady(); + follower2Node.configDataStore().waitTillReady(); + + EntityOwnershipService leaderEntityOwnershipService = newOwnershipService(leaderDistributedDataStore); + EntityOwnershipService follower1EntityOwnershipService = newOwnershipService(follower1Node.configDataStore()); + EntityOwnershipService follower2EntityOwnershipService = newOwnershipService(follower2Node.configDataStore()); + + leaderNode.kit().waitUntilLeader(leaderNode.configDataStore().getActorContext(), ENTITY_OWNERSHIP_SHARD_NAME); + + // Register leader candidate for entity1 and verify it becomes owner + + EntityOwnershipCandidateRegistration leaderEntity1Reg = leaderEntityOwnershipService.registerCandidate(ENTITY1); + + verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-1"); + verifyOwner(leaderDistributedDataStore, ENTITY1, "member-1"); + + leaderEntity1Reg.close(); + follower1EntityOwnershipService.registerCandidate(ENTITY1); + + verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-2"); + verifyOwner(leaderDistributedDataStore, ENTITY1, "member-2"); + } + + @Test + public void testOwnerSelectedOnRapidRegisteringAndUnregisteringOfCandidates() throws Exception { + String name = "test"; + MemberNode leaderNode = MemberNode.builder(memberNodes).akkaConfig("Member1").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(leaderDatastoreContextBuilder).build(); + + MemberNode follower1Node = MemberNode.builder(memberNodes).akkaConfig("Member2").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(followerDatastoreContextBuilder).build(); + + MemberNode follower2Node = MemberNode.builder(memberNodes).akkaConfig("Member3").testName(name ). + moduleShardsConfig(MODULE_SHARDS_CONFIG).schemaContext(SCHEMA_CONTEXT).createOperDatastore(false). + datastoreContextBuilder(followerDatastoreContextBuilder).build(); + + DistributedDataStore leaderDistributedDataStore = leaderNode.configDataStore(); + + leaderDistributedDataStore.waitTillReady(); + follower1Node.configDataStore().waitTillReady(); + follower2Node.configDataStore().waitTillReady(); + + EntityOwnershipService leaderEntityOwnershipService = newOwnershipService(leaderDistributedDataStore); + EntityOwnershipService follower1EntityOwnershipService = newOwnershipService(follower1Node.configDataStore()); + EntityOwnershipService follower2EntityOwnershipService = newOwnershipService(follower2Node.configDataStore()); + + leaderNode.kit().waitUntilLeader(leaderNode.configDataStore().getActorContext(), ENTITY_OWNERSHIP_SHARD_NAME); + + // Register leader candidate for entity1 and verify it becomes owner + + EntityOwnershipCandidateRegistration leaderEntity1Reg = leaderEntityOwnershipService.registerCandidate(ENTITY1); + + verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-1"); + verifyOwner(leaderDistributedDataStore, ENTITY1, "member-1"); + + follower1EntityOwnershipService.registerCandidate(ENTITY1); + leaderEntity1Reg.close(); + + verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-2"); + verifyOwner(leaderDistributedDataStore, ENTITY1, "member-2"); + } + + private static void verifyGetOwnershipState(EntityOwnershipService service, Entity entity, + boolean isOwner, boolean hasOwner) { Optional state = service.getOwnershipState(entity); assertEquals("getOwnershipState present", true, state.isPresent()); assertEquals("isOwner", isOwner, state.get().isOwner()); -- 2.36.6