[helix] branch master updated: Change IllegalStateException to Helix Exception for CRUSH based rebalance strategy algorithm

[jxue]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 362de17  Change IllegalStateException to Helix Exception for CRUSH 
based rebalance strategy algorithm
362de17 is described below

commit 362de17b9084d5eded8e54f2e71dd90118ee739d
Author: Ali Reza Zamani Zadeh Najari <anaj...@linkedin.com>
AuthorDate: Mon Jul 15 15:25:52 2019 -0700

    Change IllegalStateException to Helix Exception for CRUSH based rebalance 
strategy algorithm
    
    In this commit the IllegalStateException has been caught and HelixException 
has been thrown for the upper layer instead. The error log shows more 
meaningful exception.
    A test has been changed accordingly.
    
    This commit fixes issue #322.
---
 .../controller/rebalancer/strategy/CrushRebalanceStrategy.java | 10 +++++++++-
 .../helix/controller/stages/BestPossibleStateCalcStage.java    |  7 +++++--
 .../controller/rebalancer/TestConstraintRebalanceStrategy.java |  5 +++--
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git 
a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java
 
b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java
index cd6dde2..4c1d972 100644
--- 
a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java
+++ 
b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/strategy/CrushRebalanceStrategy.java
@@ -89,7 +89,15 @@ public class CrushRebalanceStrategy implements 
RebalanceStrategy<ResourceControl
       long data = partitionName.hashCode();
 
       // apply the placement rules
-      List<Node> selected = select(topNode, data, _replicas, eventId);
+      List<Node> selected;
+      try {
+        selected = select(topNode, data, _replicas, eventId);
+      } catch (IllegalStateException e) {
+        String errorMessage = String
+            .format("Could not select enough number of nodes. %s partition %s, 
required %d",
+                _resourceName, partitionName, _replicas);
+        throw new HelixException(errorMessage, e);
+      }
 
       if (selected.size() < _replicas) {
         LogUtil.logError(Log, eventId, String
diff --git 
a/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java
 
b/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java
index 9bbf12a..539d7d9 100644
--- 
a/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java
+++ 
b/helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java
@@ -257,9 +257,12 @@ public class BestPossibleStateCalcStage extends 
AbstractBaseStage {
 
         // Check if calculation is done successfully
         return checkBestPossibleStateCalculation(idealState);
+      } catch (HelixException e) {
+        // No eligible instance is found.
+        LogUtil.logError(logger, _eventId, e.getMessage());
       } catch (Exception e) {
-        LogUtil
-            .logError(logger, _eventId, "Error computing assignment for 
resource " + resourceName + ". Skipping.");
+        LogUtil.logError(logger, _eventId,
+            "Error computing assignment for resource " + resourceName + ". 
Skipping.");
         // TODO : remove this part after debugging NPE
         StringBuilder sb = new StringBuilder();
 
diff --git 
a/helix-core/src/test/java/org/apache/helix/controller/rebalancer/TestConstraintRebalanceStrategy.java
 
b/helix-core/src/test/java/org/apache/helix/controller/rebalancer/TestConstraintRebalanceStrategy.java
index a9e53f8..ad1f420 100644
--- 
a/helix-core/src/test/java/org/apache/helix/controller/rebalancer/TestConstraintRebalanceStrategy.java
+++ 
b/helix-core/src/test/java/org/apache/helix/controller/rebalancer/TestConstraintRebalanceStrategy.java
@@ -29,6 +29,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
+import org.apache.helix.HelixException;
 import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceHardConstraint;
 import 
org.apache.helix.api.rebalancer.constraint.AbstractRebalanceSoftConstraint;
 import 
org.apache.helix.api.rebalancer.constraint.dataprovider.CapacityProvider;
@@ -271,7 +272,7 @@ public class TestConstraintRebalanceStrategy {
           
Collections.<AbstractRebalanceHardConstraint>singletonList(capacityConstraint),
           Collections.EMPTY_LIST);
       Assert.fail("Assignment should fail because of insufficient capacity.");
-    } catch (IllegalStateException e) {
+    } catch (HelixException e) {
       // expected
     }
   }
@@ -300,7 +301,7 @@ public class TestConstraintRebalanceStrategy {
     try {
       calculateAssignment(constraints, Collections.EMPTY_LIST);
       Assert.fail("Assignment should fail because of the conflicting capacity 
constraint.");
-    } catch (IllegalStateException e) {
+    } catch (HelixException e) {
       // expected
     }
   }