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 <moraja@cisco.com>

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 ed2f4c3e492b91268729e349fef5ef008bec6870..3a66924edca7baaa926f5bee0fb54f8d8df89ee3 100644 (file)
--- 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
@@ -357,8 +357,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 7dcf9ec11e2eb9e99fad264cb8caef7b5b789f6a..f16493e93c36f671abe5bb726600eff1d144283e 100644 (file)
--- 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
@@ -715,6 +715,88 @@ public class DistributedEntityOwnershipIntegrationTest {
         });
     }
 
+    @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<EntityOwnershipState> state = service.getOwnershipState(entity);