ndimiduk commented on code in PR #6651:
URL: https://github.com/apache/hbase/pull/6651#discussion_r1965217870
##########
hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java:
##########
@@ -597,16 +643,53 @@ protected List<RegionPlan> balanceTable(TableName
tableName,
continue;
}
- cluster.doAction(action);
+ int conditionalViolationsChange = 0;
+ boolean isViolatingConditionals = false;
+ boolean moveImprovedConditionals = false;
+ // Only check conditionals if they are enabled
+ if (balancerConditionals.isConditionalBalancingEnabled()) {
+ // Always accept a conditional generator output. Sometimes conditional
generators
+ // may need to make controversial moves in order to break what would
otherwise
+ // be a deadlocked situation.
+ // Otherwise, for normal moves, evaluate the action.
+ if
(RegionPlanConditionalCandidateGenerator.class.isAssignableFrom(generator.getClass()))
{
+ conditionalViolationsChange = -1;
+ } else {
+ conditionalViolationsChange =
+ balancerConditionals.getViolationCountChange(cluster, action);
+ isViolatingConditionals = balancerConditionals.isViolating(cluster,
action);
+ }
+ moveImprovedConditionals = conditionalViolationsChange < 0;
+ if (moveImprovedConditionals) {
+ planImprovedConditionals = true;
+ }
+ }
+
+ // Change state and evaluate costs
+ try {
+ cluster.doAction(action);
+ } catch (IllegalStateException | ArrayIndexOutOfBoundsException e) {
+ LOG.warn(
+ "Generator {} produced invalid action! "
Review Comment:
You convinced me that it's useful by way of the scary message about balancer
deadlock. If that risk is real, seems like we want to notify the operator
somehow.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
