narendly commented on a change in pull request #466: Integrate the WAGED 
rebalancer with all the related components.
URL: https://github.com/apache/helix/pull/466#discussion_r324051056
 
 

 ##########
 File path: 
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
 ##########
 @@ -118,27 +110,26 @@ protected WagedRebalancer(AssignmentMetadataStore 
assignmentMetadataStore,
   public Map<String, IdealState> 
computeNewIdealStates(ResourceControllerDataProvider clusterData,
       Map<String, Resource> resourceMap, final CurrentStateOutput 
currentStateOutput)
       throws HelixRebalanceException {
-    LOG.info("Start computing new ideal states for resources: {}", 
resourceMap.keySet().toString());
+    if (resourceMap.isEmpty()) {
+      LOG.warn("There is no resource to be rebalanced by {}",
+          this.getClass().getSimpleName());
+      return Collections.emptyMap();
+    }
 
-    // Find the compatible resources: 1. FULL_AUTO 2. Configured to use the 
WAGED rebalancer
-    resourceMap = resourceMap.entrySet().stream().filter(resourceEntry -> {
+    LOG.info("Start computing new ideal states for resources: {}", 
resourceMap.keySet().toString());
+    if (!resourceMap.entrySet().stream().allMatch(resourceEntry -> {
       IdealState is = clusterData.getIdealState(resourceEntry.getKey());
       return is != null && 
is.getRebalanceMode().equals(IdealState.RebalanceMode.FULL_AUTO)
           && getClass().getName().equals(is.getRebalancerClassName());
-    }).collect(Collectors.toMap(resourceEntry -> resourceEntry.getKey(),
-        resourceEntry -> resourceEntry.getValue()));
-
-    if (resourceMap.isEmpty()) {
-      LOG.warn("There is no valid resource to be rebalanced by {}",
-          this.getClass().getSimpleName());
-      return Collections.emptyMap();
-    } else {
-      LOG.info("Valid resources that will be rebalanced by {}: {}", 
this.getClass().getSimpleName(),
-          resourceMap.keySet().toString());
+    })) {
+      throw new HelixRebalanceException(
+          "Input contains invalid resource(s) that cannot be rebalanced by the 
WAGED rebalancer.",
 
 Review comment:
   I think that the occurrence for this Exception would be low, so would it be 
a better idea to actually log the names of resources that cannot be rebalanced?
   
   From the operator's perspective, it would be very, very helpful. If I just 
saw this exception without the actual names of the resources, this error 
message is almost useless. Say you have 100 resources, and the rebalance failed 
due to 1 of them - then you would have to loop through all 100 of them just to 
find the problem child!

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