Prevent non-voting member from becoming entity owner

The use case for a non-voting member is as a backup node so
it makes sense not to allow a non-voting member to become an
entity owner. Also, when a voting state change occurs,
ownership is re-evauated for all entities.

Change-Id: Id8d75c4ae2f91be10aa20e97bdc4448776ae4445
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
(cherry picked from commit 29c8203015aa8f2891c305e82f0cf70c3de3f281)

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
index fd6eb17384448bbef1b0885fd0b99d7b4997f0d1..e3d040c993b678aec724d8ec3ed2f7542cc4a54a 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
@@ -840,6 +840,12 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
      */
     protected abstract Optional<ActorRef> getRoleChangeNotifier();
 
+    /**
+     * This method is called on the leader when a voting change operation completes.
+     */
+    protected void onVotingStateChangeComplete() {
+    }
+
     /**
      * This method is called prior to operations such as leadership transfer and actor shutdown when the leader
      * must pause or stop its duties. This method allows derived classes to gracefully pause or finish current
@@ -1031,5 +1037,4 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
             return new SimpleBehaviorState(lastValidLeaderId, lastLeaderId, behavior);
         }
     }
-
 }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupport.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupport.java
index 0014a6fce716f988a2f96ffe22b3e0ffc5ee6d2b..ee9bf5d4c802e312a014a4fe300afd26b380a6d9 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupport.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupport.java
@@ -701,6 +701,8 @@ class RaftActorServerConfigurationSupport {
             if (succeeded && localServerChangedToNonVoting) {
                 LOG.debug("Leader changed to non-voting - trying leadership transfer");
                 raftActor.becomeNonVoting();
+            } else if (raftActor.isLeader()) {
+                raftActor.onVotingStateChangeComplete();
             }
         }
 

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 56b169c5562a30f2703f8321595961e332608a05..b20d91cb76c47ee57e2265712e10f452d585f6bd 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
@@ -55,6 +55,7 @@ import org.opendaylight.controller.cluster.datastore.entityownership.messages.Un
 import org.opendaylight.controller.cluster.datastore.entityownership.messages.UnregisterListenerLocal;
 import org.opendaylight.controller.cluster.datastore.entityownership.selectionstrategy.EntityOwnerSelectionStrategy;
 import org.opendaylight.controller.cluster.datastore.entityownership.selectionstrategy.EntityOwnerSelectionStrategyConfig;
+import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier;
 import org.opendaylight.controller.cluster.datastore.messages.BatchedModifications;
 import org.opendaylight.controller.cluster.datastore.messages.PeerDown;
 import org.opendaylight.controller.cluster.datastore.messages.PeerUp;
@@ -64,6 +65,7 @@ import org.opendaylight.controller.cluster.datastore.modification.MergeModificat
 import org.opendaylight.controller.cluster.datastore.modification.Modification;
 import org.opendaylight.controller.cluster.datastore.modification.WriteModification;
 import org.opendaylight.controller.cluster.raft.RaftState;
+import org.opendaylight.controller.cluster.raft.VotingState;
 import org.opendaylight.mdsal.eos.dom.api.DOMEntity;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
@@ -376,6 +378,30 @@ class EntityOwnershipShard extends Shard {
         super.onLeaderChanged(oldLeader, newLeader);
     }
 
+    @Override
+    protected void onVotingStateChangeComplete() {
+        // Re-evaluate ownership for all entities - if a member changed from voting to non-voting it should lose
+        // ownership and vice versa it now is a candidate to become owner.
+        final List<Modification> modifications = new ArrayList<>();
+        searchForEntities((entityTypeNode, entityNode) -> {
+            YangInstanceIdentifier entityPath = YangInstanceIdentifier.builder(ENTITY_TYPES_PATH)
+                    .node(entityTypeNode.getIdentifier()).node(ENTITY_NODE_ID).node(entityNode.getIdentifier())
+                    .node(ENTITY_OWNER_NODE_ID).build();
+
+            Optional<String> possibleOwner = entityNode.getChild(ENTITY_OWNER_NODE_ID).transform(
+                node -> node.getValue().toString());
+            String newOwner = newOwner(possibleOwner.orNull(), getCandidateNames(entityNode),
+                    getEntityOwnerElectionStrategy(entityPath));
+
+            if (!newOwner.equals(possibleOwner.or(""))) {
+                modifications.add(new WriteModification(entityPath,
+                        ImmutableNodes.leafNode(ENTITY_OWNER_NODE_ID, newOwner)));
+            }
+        });
+
+        commitCoordinator.commitModifications(modifications, this);
+    }
+
     private void initializeDownPeerMemberNamesFromClusterState() {
         java.util.Optional<Cluster> cluster = getRaftActorContext().getCluster();
         if (!cluster.isPresent()) {
@@ -623,10 +649,16 @@ class EntityOwnershipShard extends Shard {
     }
 
     private Collection<String> getViableCandidates(final Collection<String> candidates) {
+        Map<MemberName, VotingState> memberToVotingState = new HashMap<>();
+        getRaftActorContext().getPeers().forEach(peerInfo -> memberToVotingState.put(
+                ShardIdentifier.fromShardIdString(peerInfo.getId()).getMemberName(), peerInfo.getVotingState()));
+
         Collection<String> viableCandidates = new ArrayList<>();
 
         for (String candidate : candidates) {
-            if (!downPeerMemberNames.contains(MemberName.forName(candidate))) {
+            MemberName memberName = MemberName.forName(candidate);
+            if (memberToVotingState.get(memberName) != VotingState.NON_VOTING
+                    && !downPeerMemberNames.contains(memberName)) {
                 viableCandidates.add(candidate);
             }
         }

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 6273e13dff49fd5e3c42987f08848b6cb07f632e..d040eb6cdef7353e02ae029351890518ff1c23d5 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
@@ -27,9 +27,12 @@ import akka.actor.ActorRef;
 import akka.actor.Status.Failure;
 import akka.actor.Status.Success;
 import akka.cluster.Cluster;
+import akka.pattern.Patterns;
 import akka.testkit.JavaTestKit;
+import akka.util.Timeout;
 import com.google.common.base.Optional;
 import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.util.concurrent.Uninterruptibles;
@@ -52,6 +55,7 @@ import org.opendaylight.controller.cluster.datastore.IntegrationTestKit;
 import org.opendaylight.controller.cluster.datastore.MemberNode;
 import org.opendaylight.controller.cluster.datastore.entityownership.selectionstrategy.EntityOwnerSelectionStrategyConfig;
 import org.opendaylight.controller.cluster.datastore.messages.AddShardReplica;
+import org.opendaylight.controller.cluster.datastore.messages.ChangeShardMembersVotingStatus;
 import org.opendaylight.controller.cluster.raft.policy.DisableElectionsRaftPolicy;
 import org.opendaylight.controller.cluster.raft.utils.InMemoryJournal;
 import org.opendaylight.controller.cluster.raft.utils.InMemorySnapshotStore;
@@ -67,6 +71,9 @@ import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import scala.concurrent.Await;
+import scala.concurrent.Future;
+import scala.concurrent.duration.FiniteDuration;
 
 /**
  * End-to-end integration tests for the entity ownership functionality.
@@ -727,6 +734,108 @@ public class DistributedEntityOwnershipIntegrationTest {
         verifyOwner(leaderDistributedDataStore, ENTITY1, "member-2");
     }
 
+    @Test
+    public void testEntityOwnershipWithNonVotingMembers() throws Exception {
+        followerDatastoreContextBuilder.shardElectionTimeoutFactor(5)
+                .customRaftPolicyImplementation(DisableElectionsRaftPolicy.class.getName());
+
+        String name = "testEntityOwnershipWithNonVotingMembers";
+        final MemberNode member1LeaderNode = MemberNode.builder(memberNodes).akkaConfig("Member1")
+                .useAkkaArtery(false).testName(name)
+                .moduleShardsConfig(MODULE_SHARDS_5_NODE_CONFIG).schemaContext(SCHEMA_CONTEXT)
+                .createOperDatastore(false).datastoreContextBuilder(leaderDatastoreContextBuilder).build();
+
+        final MemberNode member2FollowerNode = MemberNode.builder(memberNodes).akkaConfig("Member2")
+                .useAkkaArtery(false).testName(name)
+                .moduleShardsConfig(MODULE_SHARDS_5_NODE_CONFIG).schemaContext(SCHEMA_CONTEXT)
+                .createOperDatastore(false).datastoreContextBuilder(followerDatastoreContextBuilder).build();
+
+        final MemberNode member3FollowerNode = MemberNode.builder(memberNodes).akkaConfig("Member3")
+                .useAkkaArtery(false).testName(name)
+                .moduleShardsConfig(MODULE_SHARDS_5_NODE_CONFIG).schemaContext(SCHEMA_CONTEXT)
+                .createOperDatastore(false).datastoreContextBuilder(followerDatastoreContextBuilder).build();
+
+        final MemberNode member4FollowerNode = MemberNode.builder(memberNodes).akkaConfig("Member4")
+                .useAkkaArtery(false).testName(name)
+                .moduleShardsConfig(MODULE_SHARDS_5_NODE_CONFIG).schemaContext(SCHEMA_CONTEXT)
+                .createOperDatastore(false).datastoreContextBuilder(followerDatastoreContextBuilder).build();
+
+        final MemberNode member5FollowerNode = MemberNode.builder(memberNodes).akkaConfig("Member5")
+                .useAkkaArtery(false).testName(name)
+                .moduleShardsConfig(MODULE_SHARDS_5_NODE_CONFIG).schemaContext(SCHEMA_CONTEXT)
+                .createOperDatastore(false).datastoreContextBuilder(followerDatastoreContextBuilder).build();
+
+        AbstractDataStore leaderDistributedDataStore = member1LeaderNode.configDataStore();
+
+        leaderDistributedDataStore.waitTillReady();
+        member2FollowerNode.configDataStore().waitTillReady();
+        member3FollowerNode.configDataStore().waitTillReady();
+        member4FollowerNode.configDataStore().waitTillReady();
+        member5FollowerNode.configDataStore().waitTillReady();
+
+        member1LeaderNode.waitForMembersUp("member-2", "member-3", "member-4", "member-5");
+
+        final DOMEntityOwnershipService member3EntityOwnershipService =
+                newOwnershipService(member3FollowerNode.configDataStore());
+        final DOMEntityOwnershipService member4EntityOwnershipService =
+                newOwnershipService(member4FollowerNode.configDataStore());
+        final DOMEntityOwnershipService member5EntityOwnershipService =
+                newOwnershipService(member5FollowerNode.configDataStore());
+
+        newOwnershipService(member1LeaderNode.configDataStore());
+        member1LeaderNode.kit().waitUntilLeader(member1LeaderNode.configDataStore().getActorContext(),
+                ENTITY_OWNERSHIP_SHARD_NAME);
+
+        // Make member4 and member5 non-voting
+
+        Future<Object> future = Patterns.ask(leaderDistributedDataStore.getActorContext().getShardManager(),
+                new ChangeShardMembersVotingStatus(ENTITY_OWNERSHIP_SHARD_NAME,
+                        ImmutableMap.of("member-4", false, "member-5", false)), new Timeout(10, TimeUnit.SECONDS));
+        Object response = Await.result(future, FiniteDuration.apply(10, TimeUnit.SECONDS));
+        if (response instanceof Throwable) {
+            throw new AssertionError("ChangeShardMembersVotingStatus failed", (Throwable)response);
+        }
+
+        assertNull("Expected null Success response. Actual " + response, response);
+
+        // Register member4 candidate for entity1 - it should not become owner since it's non-voting
+
+        member4EntityOwnershipService.registerCandidate(ENTITY1);
+        verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-4");
+
+        // Register member5 candidate for entity2 - it should not become owner since it's non-voting
+
+        member5EntityOwnershipService.registerCandidate(ENTITY2);
+        verifyCandidates(leaderDistributedDataStore, ENTITY2, "member-5");
+
+        Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
+        verifyOwner(leaderDistributedDataStore, ENTITY1, "");
+        verifyOwner(leaderDistributedDataStore, ENTITY2, "");
+
+        // Register member3 candidate for entity1 - it should become owner since it's voting
+
+        member3EntityOwnershipService.registerCandidate(ENTITY1);
+        verifyCandidates(leaderDistributedDataStore, ENTITY1, "member-4", "member-3");
+        verifyOwner(leaderDistributedDataStore, ENTITY1, "member-3");
+
+        // Switch member4 and member5 back to voting and member3 non-voting. This should result in member4 and member5
+        // to become entity owners.
+
+        future = Patterns.ask(leaderDistributedDataStore.getActorContext().getShardManager(),
+                new ChangeShardMembersVotingStatus(ENTITY_OWNERSHIP_SHARD_NAME,
+                        ImmutableMap.of("member-3", false, "member-4", true, "member-5", true)),
+                new Timeout(10, TimeUnit.SECONDS));
+        response = Await.result(future, FiniteDuration.apply(10, TimeUnit.SECONDS));
+        if (response instanceof Throwable) {
+            throw new AssertionError("ChangeShardMembersVotingStatus failed", (Throwable)response);
+        }
+
+        assertNull("Expected null Success response. Actual " + response, response);
+
+        verifyOwner(leaderDistributedDataStore, ENTITY1, "member-4");
+        verifyOwner(leaderDistributedDataStore, ENTITY2, "member-5");
+    }
+
     private static void verifyGetOwnershipState(final DOMEntityOwnershipService service, final DOMEntity entity,
             final EntityOwnershipState expState) {
         Optional<EntityOwnershipState> state = service.getOwnershipState(entity);