BUG 5656 : Entity ownership candidates not removed consistently on leadership change
[controller.git] / opendaylight / md-sal / sal-distributed-datastore / src / main / java / org / opendaylight / controller / cluster / datastore / entityownership / EntityOwnershipShard.java
index 3a031fc6a4313b6a0177e2c8c66287ec00912676..0ca4980bbf0df4344d187b4b86f62a1a9e37126e 100644 (file)
@@ -57,7 +57,6 @@ import org.opendaylight.controller.cluster.datastore.modification.MergeModificat
 import org.opendaylight.controller.cluster.datastore.modification.WriteModification;
 import org.opendaylight.controller.md.sal.common.api.clustering.Entity;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
 import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapNode;
@@ -114,7 +113,7 @@ class EntityOwnershipShard extends Shard {
     }
 
     @Override
-    public void onReceiveCommand(final Object message) throws Exception {
+    public void handleCommand(final Object message) {
         if(message instanceof RegisterCandidateLocal) {
             onRegisterCandidateLocal((RegisterCandidateLocal) message);
         } else if(message instanceof UnregisterCandidateLocal) {
@@ -134,7 +133,7 @@ class EntityOwnershipShard extends Shard {
         } else if(message instanceof SelectOwner) {
             onSelectOwner((SelectOwner) message);
         } else if(!commitCoordinator.handleMessage(message, this)) {
-            super.onReceiveCommand(message);
+            super.handleCommand(message);
         }
     }
 
@@ -144,7 +143,7 @@ class EntityOwnershipShard extends Shard {
         String currentOwner = getCurrentOwner(selectOwner.getEntityPath());
         if(Strings.isNullOrEmpty(currentOwner)) {
             String entityType = EntityOwnersModel.entityTypeFromEntityPath(selectOwner.getEntityPath());
-            writeNewOwner(selectOwner.getEntityPath(), newOwner(selectOwner.getAllCandidates(),
+            writeNewOwner(selectOwner.getEntityPath(), newOwner(currentOwner, selectOwner.getAllCandidates(),
                     entityOwnershipStatistics.byEntityType(entityType),
                     selectOwner.getOwnerSelectionStrategy()));
 
@@ -189,16 +188,29 @@ class EntityOwnershipShard extends Shard {
 
         getSender().tell(SuccessReply.INSTANCE, getSelf());
 
-        searchForEntitiesOwnedBy(localMemberName, new EntityWalker() {
+        searchForEntities(new EntityWalker() {
             @Override
             public void onEntity(MapEntryNode entityTypeNode, MapEntryNode entityNode) {
-                Optional<DataContainerChild<? extends PathArgument, ?>> possibleType =
-                        entityTypeNode.getChild(ENTITY_TYPE_NODE_ID);
+                Optional<DataContainerChild<?, ?>> possibleType = entityTypeNode.getChild(ENTITY_TYPE_NODE_ID);
                 String entityType = possibleType.isPresent() ? possibleType.get().getValue().toString() : null;
                 if (registerListener.getEntityType().equals(entityType)) {
+                    final boolean hasOwner;
+                    final boolean isOwner;
+
+                    Optional<DataContainerChild<?, ?>> possibleOwner = entityNode.getChild(ENTITY_OWNER_NODE_ID);
+                    if (possibleOwner.isPresent()) {
+                        isOwner = localMemberName.equals(possibleOwner.get().getValue().toString());
+                        hasOwner = true;
+                    } else {
+                        isOwner = false;
+                        hasOwner = false;
+                    }
+
                     Entity entity = new Entity(entityType,
-                            (YangInstanceIdentifier) entityNode.getChild(ENTITY_ID_NODE_ID).get().getValue());
-                    listenerSupport.notifyEntityOwnershipListener(entity, false, true, true, registerListener.getListener());
+                        (YangInstanceIdentifier) entityNode.getChild(ENTITY_ID_NODE_ID).get().getValue());
+
+                    listenerSupport.notifyEntityOwnershipListener(entity, false, isOwner, hasOwner,
+                        registerListener.getListener());
                 }
             }
         });
@@ -262,15 +274,17 @@ class EntityOwnershipShard extends Shard {
                 newLeader, isLeader);
 
         if(isLeader) {
-            // We were just elected leader. If the old leader is down, select new owners for the entities
-            // owned by the down leader.
 
-            String oldLeaderMemberName = peerIdToMemberNames.get(oldLeader);
+            // Clear all existing strategies so that they get re-created when we call createStrategy again
+            // This allows the strategies to be re-initialized with existing statistics maintained by
+            // EntityOwnershipStatistics
+            strategyConfig.clearStrategies();
 
-            LOG.debug("{}: oldLeaderMemberName: {}", persistenceId(), oldLeaderMemberName);
-
-            if(downPeerMemberNames.contains(oldLeaderMemberName)) {
-                removeCandidateFromEntities(oldLeaderMemberName);
+            // Remove the candidates for all members that are known to be down. In a cluster which has greater than
+            // 3 nodes it is possible for a some node beside the leader being down when the leadership transitions
+            // it makes sense to use this event to remove all the candidates for those downed nodes
+            for(String downPeerName : downPeerMemberNames){
+                removeCandidateFromEntities(downPeerName);
             }
         } else {
             // The leader changed - notify the coordinator to check if pending modifications need to be sent.
@@ -288,7 +302,7 @@ class EntityOwnershipShard extends Shard {
             if(message.getRemovedCandidate().equals(currentOwner) || message.getRemainingCandidates().size() == 0){
                 String entityType = EntityOwnersModel.entityTypeFromEntityPath(message.getEntityPath());
                 writeNewOwner(message.getEntityPath(),
-                        newOwner(message.getRemainingCandidates(), entityOwnershipStatistics.byEntityType(entityType),
+                        newOwner(currentOwner, message.getRemainingCandidates(), entityOwnershipStatistics.byEntityType(entityType),
                                 getEntityOwnerElectionStrategy(message.getEntityPath())));
             }
         } else {
@@ -311,7 +325,7 @@ class EntityOwnershipShard extends Shard {
 
     private EntityOwnerSelectionStrategy getEntityOwnerElectionStrategy(YangInstanceIdentifier entityPath) {
         final String entityType = EntityOwnersModel.entityTypeFromEntityPath(entityPath);
-        return strategyConfig.createStrategy(entityType);
+        return strategyConfig.createStrategy(entityType, entityOwnershipStatistics.byEntityType(entityType));
     }
 
     private void onCandidateAdded(CandidateAdded message) {
@@ -336,13 +350,13 @@ class EntityOwnershipShard extends Shard {
         LOG.debug("{}: Using strategy {} to select owner", persistenceId(), strategy);
         if(Strings.isNullOrEmpty(currentOwner)){
             if(strategy.getSelectionDelayInMillis() == 0L) {
-                writeNewOwner(message.getEntityPath(), newOwner(message.getAllCandidates(),
+                writeNewOwner(message.getEntityPath(), newOwner(currentOwner, message.getAllCandidates(),
                         entityOwnershipStatistics.byEntityType(entityType), strategy));
             } else if(message.getAllCandidates().size() == availableMembers) {
                 LOG.debug("{}: Received the maximum candidates requests : {} writing new owner",
                         persistenceId(), availableMembers);
                 cancelOwnerSelectionTask(message.getEntityPath());
-                writeNewOwner(message.getEntityPath(), newOwner(message.getAllCandidates(),
+                writeNewOwner(message.getEntityPath(), newOwner(currentOwner, message.getAllCandidates(),
                         entityOwnershipStatistics.byEntityType(entityType), strategy));
             } else {
                 scheduleOwnerSelection(message.getEntityPath(), message.getAllCandidates(), strategy);
@@ -399,26 +413,6 @@ class EntityOwnershipShard extends Shard {
         return ((MapNode)entity.getChild(CANDIDATE_NODE_ID).get()).getChild(candidateNodeKey(candidateName)).isPresent();
     }
 
-    private void searchForEntitiesOwnedBy(final String owner, final EntityWalker walker) {
-        Optional<NormalizedNode<?, ?>> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH);
-        if(!possibleEntityTypes.isPresent()) {
-            return;
-        }
-
-        LOG.debug("{}: Searching for entities owned by {}", persistenceId(), owner);
-
-        searchForEntities(new EntityWalker() {
-            @Override
-            public void onEntity(MapEntryNode entityTypeNode, MapEntryNode entityNode) {
-                Optional<DataContainerChild<? extends PathArgument, ?>> possibleOwner =
-                        entityNode.getChild(ENTITY_OWNER_NODE_ID);
-                if (possibleOwner.isPresent() && owner.equals(possibleOwner.get().getValue().toString())) {
-                    walker.onEntity(entityTypeNode, entityNode);
-                }
-            }
-        });
-    }
-
     private void searchForEntities(EntityWalker walker) {
         Optional<NormalizedNode<?, ?>> possibleEntityTypes = getDataStore().readNode(ENTITY_TYPES_PATH);
         if(!possibleEntityTypes.isPresent()) {
@@ -426,10 +420,10 @@ class EntityOwnershipShard extends Shard {
         }
 
         for(MapEntryNode entityType:  ((MapNode) possibleEntityTypes.get()).getValue()) {
-            Optional<DataContainerChild<? extends PathArgument, ?>> possibleEntities =
-                    entityType.getChild(ENTITY_NODE_ID);
+            Optional<DataContainerChild<?, ?>> possibleEntities = entityType.getChild(ENTITY_NODE_ID);
             if(!possibleEntities.isPresent()) {
-                continue; // shouldn't happen but handle anyway
+                // shouldn't happen but handle anyway
+                continue;
             }
 
             for(MapEntryNode entity:  ((MapNode) possibleEntities.get()).getValue()) {
@@ -472,12 +466,12 @@ class EntityOwnershipShard extends Shard {
         }
     }
 
-    private String newOwner(Collection<String> candidates, Map<String, Long> statistics, EntityOwnerSelectionStrategy ownerSelectionStrategy) {
+    private String newOwner(String currentOwner, Collection<String> candidates, Map<String, Long> statistics, EntityOwnerSelectionStrategy ownerSelectionStrategy) {
         Collection<String> viableCandidates = getViableCandidates(candidates);
         if(viableCandidates.size() == 0){
             return "";
         }
-        return ownerSelectionStrategy.newOwner(viableCandidates, statistics);
+        return ownerSelectionStrategy.newOwner(currentOwner, viableCandidates);
     }
 
     private Collection<String> getViableCandidates(Collection<String> candidates) {