[helix] 13/15: Fix compute IdealState mapping tool

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git

commit f6f50c2ab72131be6072a74ec558a9147e979687
Author: Junkai Xue <jx...@linkedin.com>
AuthorDate: Tue Jun 18 15:38:24 2019 -0700

    Fix compute IdealState mapping tool
    
    There is a bug in IdealState mapping tool. It does not filtered out the instances are live but disabled. Add this logic and add extra tests for it.
    
    RB=1706106
    BUG=HELIX-1974
    G=helix-reviewers
    A=hulee
    
    Signed-off-by: Hunter Lee <hu...@linkedin.com>
---
 .../main/java/org/apache/helix/util/HelixUtil.java |  7 ++++
 .../helix/util/TestIdealStateAssignment.java       | 11 ++++--
 .../TestIdealStateAssignment.NoIdealState.json     | 44 +++++++++++++++++++++-
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/util/HelixUtil.java b/helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
index dafe08d..ef2e578 100644
--- a/helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
@@ -28,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 import org.apache.helix.HelixDataAccessor;
 import org.apache.helix.PropertyType;
 import org.apache.helix.controller.dataproviders.ResourceControllerDataProvider;
@@ -175,6 +176,12 @@ public final class HelixUtil {
             .getStateCountMap(liveInstances.size(), Integer.parseInt(idealState.getReplicas())),
         idealState.getMaxPartitionsPerInstance());
 
+    // Remove all disabled instances so that Helix will not consider them live.
+    List<String> disabledInstance =
+        instanceConfigs.stream().filter(enabled -> !enabled.getInstanceEnabled())
+            .map(InstanceConfig::getInstanceName).collect(Collectors.toList());
+    liveInstances.removeAll(disabledInstance);
+
     Map<String, List<String>> preferenceLists = strategy
         .computePartitionAssignment(allNodes, liveInstances,
             new HashMap<String, Map<String, String>>(), cache).getListFields();
diff --git a/helix-core/src/test/java/org/apache/helix/util/TestIdealStateAssignment.java b/helix-core/src/test/java/org/apache/helix/util/TestIdealStateAssignment.java
index 7d94b35..fa738b6 100644
--- a/helix-core/src/test/java/org/apache/helix/util/TestIdealStateAssignment.java
+++ b/helix-core/src/test/java/org/apache/helix/util/TestIdealStateAssignment.java
@@ -33,14 +33,17 @@ public class TestIdealStateAssignment {
   private static final String fileNamePrefix = "TestIdealStateAssignment";
 
   @Test(dataProvider = "IdealStateInput")
-  public void testIdealStateAssignment(String clusterName,
-      List<String> instances, List<String> partitions, String numReplicas, String stateModeDef,
-      String strategyName, Map<String, Map<String, String>> expectedMapping)
+  public void testIdealStateAssignment(String clusterName, List<String> instances,
+      List<String> partitions, String numReplicas, String stateModeDef, String strategyName,
+      Map<String, Map<String, String>> expectedMapping, List<String> disabledInstances)
       throws IllegalAccessException, InstantiationException, ClassNotFoundException {
     ClusterConfig clusterConfig = new ClusterConfig(clusterName);
     List<InstanceConfig> instanceConfigs = new ArrayList<>();
     for (String instance : instances) {
       instanceConfigs.add(new InstanceConfig(instance));
+      if (disabledInstances.contains(instance)) {
+        instanceConfigs.get(instanceConfigs.size() - 1).setInstanceEnabled(false);
+      }
     }
 
     IdealState idealState = new IdealState("TestResource");
@@ -58,7 +61,7 @@ public class TestIdealStateAssignment {
   public Object[][] getIdealStateInput() {
     final String[] inputs =
         { "ClusterName", "Instances", "Partitions", "NumReplica", "StateModelDef", "StrategyName",
-            "ExpectedMapping"
+            "ExpectedMapping", "DisabledInstances"
         };
     return TestInputLoader.loadTestInputs(fileNamePrefix + ".NoIdealState.json", inputs);
   }
diff --git a/helix-core/src/test/resources/TestIdealStateAssignment.NoIdealState.json b/helix-core/src/test/resources/TestIdealStateAssignment.NoIdealState.json
index 908c923..fc00de5 100644
--- a/helix-core/src/test/resources/TestIdealStateAssignment.NoIdealState.json
+++ b/helix-core/src/test/resources/TestIdealStateAssignment.NoIdealState.json
@@ -34,7 +34,8 @@
         "localhost_5" : "MASTER",
         "localhost_4" : "SLAVE"
       }
-    }
+    },
+    "DisabledInstances" : []
   },
   {
     "ClusterName": "TestCluster2",
@@ -79,6 +80,45 @@
         "localhost_1" : "SLAVE",
         "localhost_2" : "SLAVE"
       }
-    }
+    },
+    "DisabledInstances" : []
+  },
+  {
+    "ClusterName": "TestCluster3",
+    "Instances": [
+      "localhost_1",
+      "localhost_2",
+      "localhost_3",
+      "localhost_4",
+      "localhost_5"
+    ],
+    "Partitions": [
+      "0",
+      "1",
+      "2",
+      "3"
+    ],
+    "NumReplica": "2",
+    "StateModelDef": "MasterSlave",
+    "StrategyName": "org.apache.helix.controller.rebalancer.strategy.CrushRebalanceStrategy",
+    "ExpectedMapping" : {
+      "0" : {
+        "localhost_3" : "MASTER",
+        "localhost_5" : "SLAVE"
+      },
+      "1" : {
+        "localhost_1" : "MASTER",
+        "localhost_5" : "SLAVE"
+      },
+      "2" : {
+        "localhost_3" : "MASTER",
+        "localhost_1" : "SLAVE"
+      },
+      "3" : {
+        "localhost_5" : "MASTER",
+        "localhost_1" : "SLAVE"
+      }
+    },
+    "DisabledInstances" : ["localhost_2", "localhost_4"]
   }
 ]