Address some comments

apache · Aug 21, 2019 · 1293beb · 1293beb
1 parent 6a0007c
commit 1293beb
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 15 deletions.
diff --git a/...java/org/apache/helix/controller/rebalancer/waged/algorithm/ConstraintBasedAlgorithm.java b/...java/org/apache/helix/controller/rebalancer/waged/algorithm/ConstraintBasedAlgorithm.java
@@ -33,7 +33,6 @@
 import org.apache.helix.controller.rebalancer.waged.constraints.SoftConstraint;
 import org.apache.helix.controller.rebalancer.waged.model.AssignableNode;
 import org.apache.helix.controller.rebalancer.waged.model.AssignableReplica;
-import org.apache.helix.controller.rebalancer.waged.model.ClusterContext;
 import org.apache.helix.controller.rebalancer.waged.model.ClusterModel;
 import org.apache.helix.controller.rebalancer.waged.model.OptimalAssignment;
 import org.slf4j.Logger;
@@ -62,8 +61,7 @@ public ConstraintBasedAlgorithm(List<HardConstraint> hardConstraints,
 
   @Override
   public OptimalAssignment calculate(ClusterModel clusterModel) {
-    ClusterContext clusterContext = clusterModel.getContext();
-    OptimalAssignment optimalAssignment = new OptimalAssignment(clusterContext);
+    OptimalAssignment optimalAssignment = new OptimalAssignment(clusterModel);
     Map<String, Set<AssignableReplica>> replicasByResource = clusterModel.getAssignableReplicaMap();
     List<AssignableNode> nodes = new ArrayList<>(clusterModel.getAssignableNodes().values());
 
@@ -96,8 +94,7 @@ private Optional<AssignableNode> getNodeWithHighestPoints(AssignableReplica repl
       boolean isValid = true;
       // evaluate all hard constraints and record all the failure reasons why one assignment fails
       for (HardConstraint hardConstraint : _hardConstraints) {
-        if (!hardConstraint.isAssignmentValid(candidateNode, replica,
-            optimalAssignment.getClusterContext())) {
+        if (!hardConstraint.isAssignmentValid(candidateNode, replica, optimalAssignment.getClusterModel().getContext())) {
           hardConstraintFailures.computeIfAbsent(candidateNode, node -> new ArrayList<>())
               .add(hardConstraint);
           isValid = false;
@@ -112,7 +109,7 @@ private Optional<AssignableNode> getNodeWithHighestPoints(AssignableReplica repl
 
     Function<AssignableNode, Double> calculatePoints =
         (candidateNode) -> _softConstraints.stream().map(softConstraint -> softConstraint
-            .assignmentScore(candidateNode, replica, optimalAssignment.getClusterContext()))
+            .assignmentScore(candidateNode, replica, optimalAssignment.getClusterModel()))
             .mapToDouble(score -> score).sum();
 
     return candidateNodes.stream().max(Comparator.comparing(calculatePoints));

diff --git a/.../main/java/org/apache/helix/controller/rebalancer/waged/algorithm/RebalanceAlgorithm.java b/.../main/java/org/apache/helix/controller/rebalancer/waged/algorithm/RebalanceAlgorithm.java
@@ -23,15 +23,19 @@
 import org.apache.helix.controller.rebalancer.waged.model.OptimalAssignment;
 
 /**
- * A generic rebalance algorithm interface for the WAGED rebalancer.
- * @see <a
- *      href="Rebalance Algorithm">https://github.com/apache/helix/wiki/Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer#rebalance-algorithm-adapter</a>
+ * A generic interface used to give the optimal assignment given the runtime cluster environment.
+ *
+ * <pre>
+ * @see <a href="https://github.com/apache/helix/wiki/
+ * Design-Proposal---Weight-Aware-Globally-Even-Distribute-Rebalancer
+ * #rebalance-algorithm-adapter">Rebalance Algorithm</a>
+ * </pre>
  */
 public interface RebalanceAlgorithm {
 
   /**
    * Rebalance the Helix resource partitions based on the input cluster model.
-
+   *
    * @param clusterModel The run time cluster model that contains all necessary information
    * @return An instance of {@link OptimalAssignment}
    */

diff --git a/...e/src/main/java/org/apache/helix/controller/rebalancer/waged/model/OptimalAssignment.java b/...e/src/main/java/org/apache/helix/controller/rebalancer/waged/model/OptimalAssignment.java
@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.helix.HelixException;
 import org.apache.helix.controller.rebalancer.waged.constraints.HardConstraint;
 import org.apache.helix.model.ResourceAssignment;
 
@@ -37,17 +38,20 @@ public class OptimalAssignment {
   private Map<AssignableNode, List<AssignableReplica>> _optimalAssignment = new HashMap<>();
   private Map<AssignableReplica, Map<AssignableNode, List<HardConstraint>>> _failedAssignments =
       new HashMap<>();
-  private final ClusterContext _clusterContext;
+  private final ClusterModel _clusterModel;
 
-  public OptimalAssignment(ClusterContext clusterContext) {
-    _clusterContext = clusterContext;
+  public OptimalAssignment(ClusterModel clusterModel) {
+    _clusterModel = clusterModel;
   }
 
-  public ClusterContext getClusterContext() {
-    return _clusterContext;
+  public ClusterModel getClusterModel() {
+    return _clusterModel;
   }
 
   public Map<String, ResourceAssignment> getOptimalResourceAssignment() {
+    if (hasAnyFailure()) {
+      throw new HelixException("The rebalance algorithm failed to compute the optimal resource assignment");
+    }
     // TODO: convert the optimal assignment to map
     return Collections.emptyMap();
   }