jiajunwang 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_r314507807
 
 

 ##########
 File path: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/ClusterContext.java
 ##########
 @@ -92,7 +96,12 @@ public int getEstimatedMaxTopStateCount() {
         .getOrDefault(resourceName, Collections.emptySet());
   }
 
-  void addPartitionToFaultZone(String faultZoneId, String resourceName, String 
partition) {
+  void addAssignment(AssignableNode assignableNode, AssignableReplica 
assignableReplica) {
 
 Review comment:
   We have this method in the ClusterModel. Why we need it here?
   
   I guess you want to use ClusterContext in the OptimalAssignment, but context 
is designed to include only partial information. I think directly use the 
ClusterModel would be easier for you.
   
   Or you will need to add more and more interfaces like this one.
   
   Let me clarify a little bit more, my major concern is that, ClusterContext 
is not necessary to understand AssignableNode. If you merge it here, we won't 
need ClusterModel.
   I will agree with the change if you can merge the ClusterModel into 
ClusterContext completely. Otherwise, the current design is confusing.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

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

Reply via email to