[GitHub] [helix] i3wangyi commented on a change in pull request #381: Implement the POC work greedy constraint based algorithm

[GitBox <gi...@apache.org>]

i3wangyi commented on a change in pull request #381: Implement the POC work greedy constraint based algorithm
URL: https://github.com/apache/helix/pull/381#discussion_r311761834
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/algorithm/ConstraintBasedAlgorithm.java
 ##########
 @@ -0,0 +1,109 @@
+package org.apache.helix.controller.rebalancer.waged.algorithm;
+
+import org.apache.helix.controller.rebalancer.waged.constraints.HardConstraint;
+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;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+
+/**
+ * The algorithm is based on a given set of constraints
+ * - HardConstraint: Approve or deny the assignment given its condition, any assignment cannot bypass any "hard constraint"
+ * - SoftConstraint: Evaluate the assignment by points/rewards/scores, a higher point means a better assignment
+ * <p>
+ * The goal is to avoid all "hard constraints" while accumulating the most points(rewards) from "soft constraints"
+ */
+public class ConstraintBasedAlgorithm implements RebalanceAlgorithm {
+    private static final Logger LOG = LoggerFactory.getLogger(ConstraintBasedAlgorithm.class);
+    private final List<HardConstraint> _hardConstraints;
+    private final List<SoftConstraint> _softConstraints;
+
+    public ConstraintBasedAlgorithm(List<HardConstraint> hardConstraints, List<SoftConstraint> softConstraints) {
+        _hardConstraints = hardConstraints;
+        _softConstraints = softConstraints;
+    }
+
+    @Override
+    public OptimalAssignment calculate(ClusterModel clusterModel) {
+        OptimalAssignment currentAssignment = new OptimalAssignment();
+        Map<String, Set<AssignableReplica>> replicasByResource = clusterModel.getAssignableReplicaMap();
+        List<AssignableNode> nodes = new ArrayList<>(clusterModel.getAssignableNodes().values());
+        ClusterContext clusterContext = clusterModel.getContext();
+
+
+        for (String resource : replicasByResource.keySet()) {
+            for (AssignableReplica replica : replicasByResource.get(resource)) {
+                Optional<AssignableNode> bestNode = getNodeWithHighestPoints(replica, nodes, clusterContext, currentAssignment);
 
 Review comment:
   It's not about performance advantage but Optional makes the developer aware of the NPE risk very explicitly. If not using Optional, the private method will have a line `return null` which is quite risky! And the private method by design won't return a list of nodes

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org