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 799e11efc1c353bfd7a04cd124d6fd1c4ebf7d69..c33194d4c72e44c0b145c7e2dec5ae0beabbafd0 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
@@ -277,7 +277,7 @@ class EntityOwnershipShard extends Shard {
 
         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),

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 17f5cd3b6af139c1fbb243132aeeefc0a8bec913..69cf4e0ff9a5255ea44234b77abb29100d31425a 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;
+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.Mockito.doNothing;
 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.mockito.ArgumentCaptor;
 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.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.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;
@@ -260,6 +268,56 @@ public class DistributedEntityOwnershipIntegrationTest {
         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 void verifyGetOwnershipState(DistributedEntityOwnershipService service, Entity entity,
             boolean isOwner, boolean hasOwner) {
         Optional<EntityOwnershipState> state = service.getOwnershipState(entity);