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 0ca4980bbf0df4344d187b4b86f62a1a9e37126e..678546e1670b3d5e6d3d4367c3c6a751f3cca910 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
@@ -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 e049238e3b6e7cb5738b2513191cba0b4fa376e0..78e2c91384844f290637c8686d2753d015a641c8 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
@@ -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<EntityOwnershipState> state = service.getOwnershipState(entity);
         assertEquals("getOwnershipState present", true, state.isPresent());
         assertEquals("isOwner", isOwner, state.get().isOwner());