BUG 4554 : Ownership is not cleared when all candidates are removed

When all candidates for an entity get unregistered at approximately
the same time it can create a situation where the owner for the
entity is not cleared. Consequently no entity ownership change is
raised where hasOwner is false even when there are no owners for
the entity.

This could be a problem for applications which do
some action when there are no candidates for an entity. The
openflow application for example relies on the disappearance of
all owners to actually remove a switch from inventory. Without
this event we have the situation that nodes hang around in inventory.

Problem Sequence
----------------

The sequence of events which leads to this problem are as follows.

Let's say member-1 owned entity-1 and there are 3 candidates for
entity-1 - member-1, member-2 and member-3. Now let's say due to
some event all candidates have to unregister. The data
transaformations will go like this.

delete member-1
delete member-2
delete member-3
delete member-1 succeeds so choose new owner - in this case member-2
make-owner member-2
delete member-2 succeeds - member-2 is not the current owner so do nothing
delete member-3 succeeds - member-3 is not the current owner so do nothing
make-owner member-2 succeeds. Now we have an owner for entity-1 even though we have not candidates

Solution
--------

The solution proposed in this patch is to set member to empty when
there are no remaining candidates. This changes the above sequence as follows.

delete member-1
delete member-2
delete member-3
delete member-1 succeeds so choose new owner - in this case member-2
make-owner member-2
delete member-2 succeeds - member-2 is not the current owner so do nothing
delete member-3 succeeds - member-3 is the last candidate so set member to ""
make-owner ""
make-owner member-2 succeeds. Now we have an owner for entity-1 even though we have not candidates
make-owner "" succeeds. Now we have owner for entity-1 set to no one as it should be

Change-Id: I583e8c6991742ada5846e87da35db255eeed144e
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 d104222b9ce8d68685ddb8d859189a52930af13c..058ab498d965cfba3168fdfaf2cb6860bf2bce48 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
@@ -273,7 +273,7 @@ class EntityOwnershipShard extends Shard {
 
         if(isLeader()) {
             String currentOwner = getCurrentOwner(message.getEntityPath());
 
         if(isLeader()) {
             String currentOwner = getCurrentOwner(message.getEntityPath());

-            if(message.getRemovedCandidate().equals(currentOwner)){
+            if(message.getRemovedCandidate().equals(currentOwner) || message.getRemainingCandidates().size() == 0){

                 String entityType = EntityOwnersModel.entityTypeFromEntityPath(message.getEntityPath());
                 writeNewOwner(message.getEntityPath(),
                         newOwner(message.getRemainingCandidates(), entityOwnershipStatistics.byEntityType(entityType),
                 String entityType = EntityOwnersModel.entityTypeFromEntityPath(message.getEntityPath());
                 writeNewOwner(message.getEntityPath(),
                         newOwner(message.getRemainingCandidates(), entityOwnershipStatistics.byEntityType(entityType),

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 634b92cdb7bc708ae7c564e372aca110938ba74b..2ca128271e0497d17446e742d6b847af905d598f 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
@@ -8,8 +8,11 @@
 package org.opendaylight.controller.cluster.datastore.entityownership;
 
 import static org.junit.Assert.assertEquals;
 package org.opendaylight.controller.cluster.datastore.entityownership;
 
 import static org.junit.Assert.assertEquals;

+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;

 import static org.mockito.AdditionalMatchers.or;
 import static org.mockito.Mockito.atMost;
 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;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;


@@ -33,14 +36,19 @@ import java.util.concurrent.TimeUnit;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;

+import org.mockito.ArgumentCaptor;

 import org.mockito.Mock;
 import org.mockito.Mock;

+import org.mockito.Mockito;

 import org.mockito.MockitoAnnotations;
 import org.opendaylight.controller.cluster.datastore.DatastoreContext;
 import org.opendaylight.controller.cluster.datastore.DistributedDataStore;
 import org.opendaylight.controller.cluster.datastore.IntegrationTestKit;
 import org.opendaylight.controller.cluster.datastore.entityownership.selectionstrategy.EntityOwnerSelectionStrategyConfig;
 import org.opendaylight.controller.md.cluster.datastore.model.SchemaContextHelper;
 import org.mockito.MockitoAnnotations;
 import org.opendaylight.controller.cluster.datastore.DatastoreContext;
 import org.opendaylight.controller.cluster.datastore.DistributedDataStore;
 import org.opendaylight.controller.cluster.datastore.IntegrationTestKit;
 import org.opendaylight.controller.cluster.datastore.entityownership.selectionstrategy.EntityOwnerSelectionStrategyConfig;
 import org.opendaylight.controller.md.cluster.datastore.model.SchemaContextHelper;

+import org.opendaylight.controller.md.sal.common.api.clustering.CandidateAlreadyRegisteredException;

 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;
 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;

+import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipCandidateRegistration;
+import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipChange;

 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipListener;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipState;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.clustering.entity.owners.rev150804.entity.owners.entity.type.entity.Candidate;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipListener;
 import org.opendaylight.controller.md.sal.common.api.clustering.EntityOwnershipState;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.clustering.entity.owners.rev150804.entity.owners.entity.type.entity.Candidate;


@@ -260,6 +268,56 @@ public class DistributedEntityOwnershipIntegrationTest {
         verify(follower1MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY2, false, false, false));
     }
 
         verify(follower1MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY2, false, false, false));
     }
 

+    /**
+     * Reproduces bug <a href="https://bugs.opendaylight.org/show_bug.cgi?id=4554">4554</a>
+     *
+     * @throws CandidateAlreadyRegisteredException
+     */
+    @Test
+    public void testCloseCandidateRegistrationInQuickSuccession() throws CandidateAlreadyRegisteredException {
+        initDatastores("testCloseCandidateRegistrationInQuickSuccession");
+
+        leaderEntityOwnershipService.registerListener(ENTITY_TYPE1, leaderMockListener);
+        follower1EntityOwnershipService.registerListener(ENTITY_TYPE1, follower1MockListener);
+        follower2EntityOwnershipService.registerListener(ENTITY_TYPE1, follower2MockListener);
+
+        final EntityOwnershipCandidateRegistration candidate1 = leaderEntityOwnershipService.registerCandidate(ENTITY1);
+        final EntityOwnershipCandidateRegistration candidate2 = follower1EntityOwnershipService.registerCandidate(ENTITY1);
+        final EntityOwnershipCandidateRegistration candidate3 = follower2EntityOwnershipService.registerCandidate(ENTITY1);
+
+        verify(leaderMockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY1, false, true, true));
+
+        Mockito.reset(leaderMockListener);
+
+        candidate1.close();
+        candidate2.close();
+        candidate3.close();
+
+        ArgumentCaptor<EntityOwnershipChange> leaderChangeCaptor = ArgumentCaptor.forClass(EntityOwnershipChange.class);
+        ArgumentCaptor<EntityOwnershipChange> follower1ChangeCaptor = ArgumentCaptor.forClass(EntityOwnershipChange.class);
+        ArgumentCaptor<EntityOwnershipChange> follower2ChangeCaptor = ArgumentCaptor.forClass(EntityOwnershipChange.class);
+        doNothing().when(leaderMockListener).ownershipChanged(leaderChangeCaptor.capture());
+        doNothing().when(follower1MockListener).ownershipChanged(follower1ChangeCaptor.capture());
+        doNothing().when(follower2MockListener).ownershipChanged(follower2ChangeCaptor.capture());
+
+        boolean passed = false;
+        for(int i=0;i<100;i++) {
+            Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS);
+            if(!leaderEntityOwnershipService.getOwnershipState(ENTITY1).get().hasOwner() &&
+                    !follower1EntityOwnershipService.getOwnershipState(ENTITY1).get().hasOwner() &&
+                    !follower2EntityOwnershipService.getOwnershipState(ENTITY1).get().hasOwner()) {
+                passed = true;
+                break;
+            }
+        }
+
+        assertTrue("No ownership change message was sent with hasOwner=false", passed);
+
+        assertFalse(leaderChangeCaptor.getAllValues().get(leaderChangeCaptor.getAllValues().size()-1).hasOwner());
+        assertFalse(follower1ChangeCaptor.getAllValues().get(follower1ChangeCaptor.getAllValues().size()-1).hasOwner());
+        assertFalse(follower2ChangeCaptor.getAllValues().get(follower2ChangeCaptor.getAllValues().size()-1).hasOwner());
+    }
+

     private static void verifyGetOwnershipState(DistributedEntityOwnershipService service, Entity entity,
             boolean isOwner, boolean hasOwner) {
         Optional<EntityOwnershipState> state = service.getOwnershipState(entity);
     private static void verifyGetOwnershipState(DistributedEntityOwnershipService service, Entity entity,
             boolean isOwner, boolean hasOwner) {
         Optional<EntityOwnershipState> state = service.getOwnershipState(entity);