You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2021/06/05 01:29:16 UTC
[GitHub] [hbase] clarax opened a new pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
clarax opened a new pull request #3356:
URL: https://github.com/apache/hbase/pull/3356
--
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
[GitHub] [hbase] clarax commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-868101875
Rebase from master to solve the test failures.
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r647684738
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -555,10 +564,10 @@ private String functionCost() {
for (CostFunction c : costFunctions) {
builder.append(c.getClass().getSimpleName());
builder.append(" : (");
- if (c.isNeeded()) {
- builder.append(c.getMultiplier());
+ if (c.isNeeded() || c.getMultiplier() > 0) {
+ builder.append("multiplier=" + c.getMultiplier());
Review comment:
This smells like an error in how `isNeeded` gets implemented. It should be taking into account things like "the multiplier isn't >0". I don't think this is an issue with your specific PR since it happens all over the `StochasticLoadBalancer` class. Something to clean up later maybe.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -331,27 +334,28 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
- sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ if (cost < minCostNeedBalance)
+ {
+ LOG.debug("Imbalance of {} on the scale of [0, 1] is {} < threshold ({}).",
+ c.getClass().getSimpleName(), cost, minCostNeedBalance);
+ }
Review comment:
What's the goal here? It looks like the trace level output of the `functionCost()` call has been removed in favor of debug output here? Will need to include the weights and output for cost functions that were over the threshold otherwise there'll be no way to reproduce why the balancer decided to skip from just the logs.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +475,19 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new moving plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed imbalance of {}" +
+ " to a new imbalance of {}. ",
+ endTime - startTime, step, plans.size(), initCost / sumMultiplier, currentCost / sumMultiplier);
sendRegionPlansToRingBuffer(plans, currentCost, initCost, initFunctionTotalCosts, step);
+ LOG.info("Moving plan submitted. Balancer is going into sleep until next period");
return plans;
}
- LOG.info("Could not find a better load balance plan. Tried {} different configurations in " +
- "{}, and did not find anything with a computed cost less than {}", step,
- java.time.Duration.ofMillis(endTime - startTime), initCost);
+ LOG.info("Could not find a better moving plan. Tried {} different configurations in " +
+ "{} ms, and did not find anything with a computed cost less than {}", step,
+ endTime - startTime, initCost);
Review comment:
Shouldn't this talk about "imbalance" score or some such instead of "computed cost"?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +475,19 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new moving plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed imbalance of {}" +
+ " to a new imbalance of {}. ",
+ endTime - startTime, step, plans.size(), initCost / sumMultiplier, currentCost / sumMultiplier);
sendRegionPlansToRingBuffer(plans, currentCost, initCost, initFunctionTotalCosts, step);
+ LOG.info("Moving plan submitted. Balancer is going into sleep until next period");
Review comment:
I like this phrasing for distinguishing that the balancer came up with the set of moves to do but did not actually do the moving itself.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -331,27 +334,28 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
- sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ if (cost < minCostNeedBalance)
+ {
+ LOG.debug("Imbalance of {} on the scale of [0, 1] is {} < threshold ({}).",
+ c.getClass().getSimpleName(), cost, minCostNeedBalance);
+ }
+ total += cost * multiplier;
}
boolean balanced = total <= 0 || sumMultiplier <= 0 ||
(sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
- }
+ LOG.info("{} {} ", isByTable ? String.format("table (%s)", tableName) : "cluster",
+ balanced ? "Skipping load balancing because weighted average imbalance= "
+ + total / sumMultiplier + "< threshold (" + minCostNeedBalance + ") on the scale of [0, 1]."
+ + "If you want more aggressive balancing, either lower threshold minCostNeedbalance ("
+ + minCostNeedBalance + ") or increase the relative weight(s) of the specific cost function(s)."
+ : "Calculating plan. May take up to " + maxRunningTime + " ms to complete.");
+
Review comment:
oof. That's a lot of logic packed into string concatenation. Can we unwrap this a bit so that future folks don't have to decipher so much?
Something like
```
if (balanced) {
LOG.info("{} - skipping load balancing because weighted average imbalance = {} > threshold ({})....",
isByTable ? "Table specific ("+tableName+")" : "Cluster wide", total/sumMultiplier, minCostNeedBalance,...);
} else {
LOG.info("{} - Calculating plan. may take up to {}ms to complete.",
isByTable ? "Table specific ("+tableName+")" : "Cluster wide", maxRunningTime);
}
```
could be rolled into the if block that calls `sendRejectionReasonToRingBuffer`?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -331,27 +334,28 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
- sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ if (cost < minCostNeedBalance)
+ {
+ LOG.debug("Imbalance of {} on the scale of [0, 1] is {} < threshold ({}).",
+ c.getClass().getSimpleName(), cost, minCostNeedBalance);
+ }
+ total += cost * multiplier;
}
boolean balanced = total <= 0 || sumMultiplier <= 0 ||
(sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
- }
+ LOG.info("{} {} ", isByTable ? String.format("table (%s)", tableName) : "cluster",
+ balanced ? "Skipping load balancing because weighted average imbalance= "
+ + total / sumMultiplier + "< threshold (" + minCostNeedBalance + ") on the scale of [0, 1]."
+ + "If you want more aggressive balancing, either lower threshold minCostNeedbalance ("
+ + minCostNeedBalance + ") or increase the relative weight(s) of the specific cost function(s)."
Review comment:
you refer to `minCostBalance` here but "threshold" above. If we're going to tell someone to change the threshold we should refer to a config that they can alter. Also which cost functions? How would I know what they are and how do I change them?
This feels like a place where we'd be better off providing a pointer to something in the ref guide that can go into all this detail instead of putting it into the log file repeatedly when the cluster is in steady state.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657532279
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -285,14 +286,12 @@ private boolean areSomeRegionReplicasColocated(BalancerClusterState c) {
return false;
}
- private String getBalanceReason(double total, double sumMultiplier) {
+ private String getBalanceReason(double total) {
if (total <= 0) {
- return "(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern) = " + total + " <= 0";
- } else if (sumMultiplier <= 0) {
- return "sumMultiplier = " + sumMultiplier + " <= 0";
+ return "(weighted sum of imbalance = " + total + " <= 0";
} else if ((total / sumMultiplier) < minCostNeedBalance) {
- return "[(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern)]/sumMultiplier = " +
- (total / sumMultiplier) + " <= minCostNeedBalance(" + minCostNeedBalance + ")";
+ return "(weighted average imbalance = " +
+ (total / sumMultiplier) + " < threshold (" + minCostNeedBalance + ")";
Review comment:
If it is 0, the cost function is excluded at constructor.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657546722
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
Review comment:
Updated log from test run: skipping load balancing because weighted average imbalance=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced); > threshold(0.06013745704467354).functionCost=1.0. If you want more aggressive balancing, either lower hbase.master.balancer.stochastic.minCostNeedBala
nce from {}or increase the relative multiplier(s) of the specific cost function(s).
--
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
[GitHub] [hbase] clarax commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-865302988
Any more concerns? @busbey @saintstack
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r647719707
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -555,10 +564,10 @@ private String functionCost() {
for (CostFunction c : costFunctions) {
builder.append(c.getClass().getSimpleName());
builder.append(" : (");
- if (c.isNeeded()) {
- builder.append(c.getMultiplier());
+ if (c.isNeeded() || c.getMultiplier() > 0) {
+ builder.append("multiplier=" + c.getMultiplier());
Review comment:
Actually this is not needed. There are some confusion in other part of the code on negative multipliers but it shouldn't happen. Will clean up.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856632369
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 9s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | master passed |
| +1 :green_heart: | compile | 0m 19s | master passed |
| +1 :green_heart: | shadedjars | 8m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 16s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 3s | the patch passed |
| +1 :green_heart: | compile | 0m 20s | the patch passed |
| +1 :green_heart: | javac | 0m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 14s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 4m 8s | hbase-balancer in the patch failed. |
| | | 34m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 856abb140302 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/testReport/ |
| Max. process+thread count | 229 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646840748
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
+ isByTable ? String.format("table (%s)", tableName) : "cluster",
Review comment:
It only prints is by table is turned on. otherwise it only prints cluster.
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r647676537
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
For the Stochastic Load Balancer isn't the only way to represent that dumping the cost functions? We do that at the more granular log level. You thinking we need to do it all the time @saintstack ?
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657550971
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
+ } else {
+ LOG.info("{} - Calculating plan. may take up to {}ms to complete.",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", maxRunningTime);
Review comment:
From test run: Cluster wide - Calculating plan. may take up to 90000ms to complete.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646864358
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
Review comment:
Because we want to expose it. and this wouldn't cause log spam.
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646628873
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
Review comment:
It's not accurate to say we're executing the plan at this point though.
What are we trying to convey with this addition? From the issue description I think it's that the act of selecting a plan for the cluster does not mean we've executed that plan. is that correct?
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-855166735
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 32s | master passed |
| +1 :green_heart: | compile | 0m 22s | master passed |
| +1 :green_heart: | shadedjars | 8m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 14s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 6m 14s | hbase-balancer in the patch passed. |
| | | 35m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3a6952c98bd3 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1ecff8a390 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/testReport/ |
| Max. process+thread count | 331 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-855166882
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | checkstyle | 0m 16s | master passed |
| +1 :green_heart: | spotbugs | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 35s | the patch passed |
| +1 :green_heart: | compile | 0m 31s | the patch passed |
| +1 :green_heart: | javac | 0m 31s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 15s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 7s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 0m 44s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 36m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b5afbfc4dfb3 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1ecff8a390 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] saintstack commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646688776
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
I'm thinking of the times when balancer says the cluster is balanced but when when the operator looks at the Master UI, it plainly is not.... For example, if the regions-per-server are not aligned, this message would explain why it thinks the cluster balanced though to the the operator, the cluster looks off.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-872911228
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 38s | master passed |
| +1 :green_heart: | compile | 3m 38s | master passed |
| +1 :green_heart: | checkstyle | 1m 17s | master passed |
| +1 :green_heart: | spotbugs | 2m 33s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 33s | the patch passed |
| +1 :green_heart: | compile | 3m 41s | the patch passed |
| +1 :green_heart: | javac | 3m 41s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 17s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 0s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 57s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 49m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 17f81ce58e63 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5ef5e64010 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646840065
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
There is a function that explain the reason of skipping. But we need to provide the big picture why t stops here. Will update.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646864080
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
Review comment:
Good callout. will massage it. and will show the example logs.
--
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
[GitHub] [hbase] clarax commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-877807358
@busbey is it good to go?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657441767
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
Review comment:
Do we have a guard somewhere against all the multipliers being 0?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -285,14 +286,12 @@ private boolean areSomeRegionReplicasColocated(BalancerClusterState c) {
return false;
}
- private String getBalanceReason(double total, double sumMultiplier) {
+ private String getBalanceReason(double total) {
if (total <= 0) {
- return "(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern) = " + total + " <= 0";
- } else if (sumMultiplier <= 0) {
- return "sumMultiplier = " + sumMultiplier + " <= 0";
+ return "(weighted sum of imbalance = " + total + " <= 0";
} else if ((total / sumMultiplier) < minCostNeedBalance) {
- return "[(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern)]/sumMultiplier = " +
- (total / sumMultiplier) + " <= minCostNeedBalance(" + minCostNeedBalance + ")";
+ return "(weighted average imbalance = " +
+ (total / sumMultiplier) + " < threshold (" + minCostNeedBalance + ")";
Review comment:
Do we have a guard somewhere against all the multipliers being 0?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +417,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance ="
+ + currentCost / sumMultiplier + ", functionCost=" + functionCost() + " computedMaxSteps: "
+ + computedMaxSteps);
Review comment:
why is `currentCost` divided by `sumMultiplier` here? isn't `currentCost` by itself already the weighted average measure of imbalance?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
Review comment:
When an operator sees this message and wants to "lower minCostNeedBalance" how do they do that?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +465,19 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new moving plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed imbalance of {}" +
+ " to a new imbalance of {}. ",
+ endTime - startTime, step, plans.size(), initCost / sumMultiplier, currentCost / sumMultiplier);
sendRegionPlansToRingBuffer(plans, currentCost, initCost, initFunctionTotalCosts, step);
+ LOG.info("Moving plan submitted. Balancer is going into sleep until next period");
return plans;
}
- LOG.info("Could not find a better load balance plan. Tried {} different configurations in " +
- "{}, and did not find anything with a computed cost less than {}", step,
- java.time.Duration.ofMillis(endTime - startTime), initCost);
+ LOG.info("Could not find a better moving plan. Tried {} different configurations in " +
+ "{} ms, and did not find anything with an imbalance score less than {}", step,
+ endTime - startTime, initCost / sumMultiplier);
+ LOG.info("Balancer is going into sleep until next period");
Review comment:
can we say in this message approx how long that will be?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +417,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance ="
+ + currentCost / sumMultiplier + ", functionCost=" + functionCost() + " computedMaxSteps: "
+ + computedMaxSteps);
Review comment:
please use parameter substitution
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646864645
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
+ isByTable ? String.format("table (%s)", tableName) : "cluster",
+ total, sumMultiplier, minCostNeedBalance, maxRunningTime / 1000);
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Balance decision detailed function costs={}", functionCost());
}
Review comment:
That makes sense. yes.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-867438899
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 3s | master passed |
| +1 :green_heart: | compile | 3m 58s | master passed |
| +1 :green_heart: | checkstyle | 1m 19s | master passed |
| +1 :green_heart: | spotbugs | 2m 41s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| +1 :green_heart: | compile | 3m 42s | the patch passed |
| -0 :warning: | javac | 0m 31s | hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17) |
| -0 :warning: | checkstyle | 0m 14s | hbase-balancer: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0) |
| -0 :warning: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | hadoopcheck | 18m 30s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 6s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 24s | The patch does not generate ASF License warnings. |
| | | 54m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 3d1ab98f36b2 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcd0fb8b1d |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-general-check/output/whitespace-eol.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856632877
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 48s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 6s | master passed |
| +1 :green_heart: | compile | 0m 29s | master passed |
| +1 :green_heart: | shadedjars | 9m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 14s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 18s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 3m 12s | hbase-balancer in the patch failed. |
| | | 34m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2d89b751df65 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/testReport/ |
| Max. process+thread count | 312 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657546722
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
Review comment:
Updated log from test run: skipping load balancing because weighted average imbalance=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced); > threshold(0.06013745704467354).functionCost=1.0. If you want more aggressive balancing, either lower hbase.master.balancer.stochastic.minCostNeedBala
nce from {}or increase the relative multiplier(s) of the specific cost function(s)
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-867905822
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 52s | master passed |
| +1 :green_heart: | compile | 3m 58s | master passed |
| +1 :green_heart: | checkstyle | 1m 20s | master passed |
| +1 :green_heart: | spotbugs | 2m 45s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 45s | the patch passed |
| +1 :green_heart: | compile | 3m 57s | the patch passed |
| -0 :warning: | javac | 0m 33s | hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17) |
| -0 :warning: | checkstyle | 0m 15s | hbase-balancer: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0) |
| -0 :warning: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | hadoopcheck | 18m 6s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 23s | The patch does not generate ASF License warnings. |
| | | 52m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux a6d9e820410e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-general-check/output/whitespace-eol.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-867971504
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 44s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed |
| +1 :green_heart: | shadedjars | 8m 22s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 20s | the patch passed |
| +1 :green_heart: | javac | 1m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 7m 11s | hbase-balancer in the patch passed. |
| +1 :green_heart: | unit | 131m 47s | hbase-server in the patch passed. |
| | | 171m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 28e532ecfd93 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/testReport/ |
| Max. process+thread count | 3431 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657551561
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +417,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance ="
Review comment:
Start StochasticLoadBalancer.balancer, initial weighted average imbalance=0.06013745704467354, functionCost=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced); computedMaxSteps=3200
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-868179471
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 37s | master passed |
| +1 :green_heart: | compile | 1m 20s | master passed |
| +1 :green_heart: | shadedjars | 8m 14s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 39s | the patch passed |
| +1 :green_heart: | compile | 1m 20s | the patch passed |
| +1 :green_heart: | javac | 1m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 11s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 7m 27s | hbase-balancer in the patch passed. |
| +1 :green_heart: | unit | 131m 41s | hbase-server in the patch passed. |
| | | 170m 51s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4a765f78efed 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/testReport/ |
| Max. process+thread count | 3770 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-868177680
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 12s | master passed |
| +1 :green_heart: | compile | 1m 33s | master passed |
| +1 :green_heart: | shadedjars | 8m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 14s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 6m 10s | hbase-balancer in the patch passed. |
| +1 :green_heart: | unit | 124m 50s | hbase-server in the patch passed. |
| | | 164m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 09f285184e01 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/testReport/ |
| Max. process+thread count | 3524 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657586765
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -285,14 +286,12 @@ private boolean areSomeRegionReplicasColocated(BalancerClusterState c) {
return false;
}
- private String getBalanceReason(double total, double sumMultiplier) {
+ private String getBalanceReason(double total) {
if (total <= 0) {
- return "(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern) = " + total + " <= 0";
- } else if (sumMultiplier <= 0) {
- return "sumMultiplier = " + sumMultiplier + " <= 0";
+ return "(weighted sum of imbalance = " + total + " <= 0";
} else if ((total / sumMultiplier) < minCostNeedBalance) {
- return "[(cost1*multiplier1)+(cost2*multiplier2)+...+(costn*multipliern)]/sumMultiplier = " +
- (total / sumMultiplier) + " <= minCostNeedBalance(" + minCostNeedBalance + ")";
+ return "(weighted average imbalance = " +
+ (total / sumMultiplier) + " < threshold (" + minCostNeedBalance + ")";
Review comment:
If they’re all 0 won’t sumMultiplier be 0 here?
--
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
[GitHub] [hbase] saintstack merged pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #3356:
URL: https://github.com/apache/hbase/pull/3356
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-867512150
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 30s | master passed |
| +1 :green_heart: | compile | 1m 34s | master passed |
| +1 :green_heart: | shadedjars | 8m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 17s | the patch passed |
| +1 :green_heart: | compile | 1m 31s | the patch passed |
| +1 :green_heart: | javac | 1m 31s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 5m 57s | hbase-balancer in the patch passed. |
| -1 :x: | unit | 123m 48s | hbase-server in the patch failed. |
| | | 163m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 80ec19cedfa6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcd0fb8b1d |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/testReport/ |
| Max. process+thread count | 3300 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/5/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-857258703
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 56s | master passed |
| +1 :green_heart: | compile | 0m 35s | master passed |
| +1 :green_heart: | checkstyle | 0m 16s | master passed |
| +1 :green_heart: | spotbugs | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 34s | the patch passed |
| +1 :green_heart: | compile | 0m 33s | the patch passed |
| -0 :warning: | javac | 0m 33s | hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17) |
| -0 :warning: | checkstyle | 0m 15s | hbase-balancer: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) |
| -0 :warning: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | hadoopcheck | 18m 4s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 0m 46s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 37m 0s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 349d2c71cb57 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-general-check/output/whitespace-eol.txt |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646624197
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
Review comment:
This is a potential value to fill into a parameter in the first string given to `LOG.info`, does having a parameter to expand in here work?
maybe a comment on this PR with some examples of the log messages we end up with post-patch would help?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
Review comment:
What's the rationale for moving this from debug logging to info logging?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
you mean beyond the given cost, multiplier, and needed threshold? you thinking of the individual cost functions?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
Review comment:
It's not accurate to say we're executing the plan at this point though.
What ware we trying to convey with this addition? From the issue description I think it's that the act of selecting a plan for the cluster does not mean we've executed that plan. is that correct?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
+ isByTable ? String.format("table (%s)", tableName) : "cluster",
+ total, sumMultiplier, minCostNeedBalance, maxRunningTime / 1000);
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Balance decision detailed function costs={}", functionCost());
}
Review comment:
if the general status message of "I'm doing some balancing (or not)" is going to info level, should this detail of individual cost functions go to debug?
--
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
[GitHub] [hbase] clarax commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-865302988
Any more concerns? @busbey @saintstack
--
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
[GitHub] [hbase] busbey commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-878491162
thanks for the merge Stack.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657532327
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
Review comment:
see above
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-857257106
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 36s | master passed |
| +1 :green_heart: | compile | 0m 20s | master passed |
| +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 16s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 7m 12s | hbase-balancer in the patch passed. |
| | | 33m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 15dbb0d4bf71 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/testReport/ |
| Max. process+thread count | 355 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-868663327
@busbey good to go?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-855166549
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | master passed |
| +1 :green_heart: | compile | 0m 20s | master passed |
| +1 :green_heart: | shadedjars | 8m 26s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 19s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| +1 :green_heart: | compile | 0m 20s | the patch passed |
| +1 :green_heart: | javac | 0m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 7m 16s | hbase-balancer in the patch passed. |
| | | 34m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 85ce74bc1cf4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1ecff8a390 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/testReport/ |
| Max. process+thread count | 354 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856634564
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 0m 32s | master passed |
| +1 :green_heart: | checkstyle | 0m 16s | master passed |
| +1 :green_heart: | spotbugs | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| -1 :x: | compile | 0m 25s | hbase-balancer in the patch failed. |
| -0 :warning: | javac | 0m 25s | hbase-balancer in the patch failed. |
| -0 :warning: | checkstyle | 0m 14s | hbase-balancer: The patch generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 15s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 0m 45s | the patch passed |
||| _ Other Tests _ |
| +0 :ok: | asflicense | 0m 27s | ASF License check generated no output? |
| | | 37m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f9590e90f875 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-general-check/output/patch-compile-hbase-balancer.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-general-check/output/patch-compile-hbase-balancer.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856952302
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 46s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 45s | master passed |
| +1 :green_heart: | compile | 0m 27s | master passed |
| +1 :green_heart: | shadedjars | 11m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 56s | the patch passed |
| +1 :green_heart: | compile | 0m 25s | the patch passed |
| +1 :green_heart: | javac | 0m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 11m 27s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 4m 23s | hbase-balancer in the patch failed. |
| | | 42m 31s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4beb83704ee7 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/testReport/ |
| Max. process+thread count | 311 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657532632
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +417,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance ="
+ + currentCost / sumMultiplier + ", functionCost=" + functionCost() + " computedMaxSteps: "
+ + computedMaxSteps);
Review comment:
No, it is the weighted sum.
--
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
[GitHub] [hbase] saintstack commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r668085556
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +303,55 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
Review comment:
good
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +303,55 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
Review comment:
And idea is that a rebalance would make it so the load is distributed?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +424,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance={}, "
Review comment:
Could we say anything here on what these values mean ? For the operator? (Need a ',' after the functionCost).
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +303,55 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0) {
+ LOG.error("At least one cost function needs a multiplier > 0. For example, set "
Review comment:
Good
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +424,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance={}, "
Review comment:
imbalance?
(Looked it up... "lack of proportion or relation between corresponding things." <= Nice)
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +303,55 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0) {
+ LOG.error("At least one cost function needs a multiplier > 0. For example, set "
+ + "hbase.master.balancer.stochastic.regionCountCost to a positive value or default");
+ return false;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() ->
+ getBalanceReason(calculatedTotal, sumMultiplier), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} <= "
+ + "threshold({}). If you want more aggressive balancing, either lower "
+ + "hbase.master.balancer.stochastic.minCostNeedBalance from {} or increase the relative "
+ + "multiplier(s) of the specific cost function(s). functionCost={}",
Review comment:
Good
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +302,54 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0)
+ {
+ LOG.error("At least one cost function needs a multiplier > 0");
+ return false;
Review comment:
What about Seans' comment above?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
+ endTime - startTime, step, plans.size(), initCost, currentCost);
sendRegionPlansToRingBuffer(plans, currentCost, initCost, initFunctionTotalCosts, step);
return plans;
}
LOG.info("Could not find a better load balance plan. Tried {} different configurations in " +
Review comment:
Was this addressed?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
Review comment:
This addressed?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
Review comment:
This addressed?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
I was thinking either explaining terms or pointing to doc or elsewhere where terms are explained and how the balancer machinations engage w/ them. Can do in a later PR, yes. This one is a big improvement as is.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657551561
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +417,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance ="
Review comment:
Start StochasticLoadBalancer.balancer, initial weighted average imbalance=0.06013745704467354, functionCost=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced); , computedMaxSteps=3200
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657546722
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
Review comment:
Updated log from test run: Cluster wide - skipping load balancing because weighted average imbalance=0.02843086355657742 <= threshold(0.05). If you want more aggressive balancing, either lower hbase.master.balancer.stochastic.minCostNeedBalance from 0.05 or increase the relative multiplier(s) of the specific cost function(s). functionCost=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=0.4727646454265159); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFun
ction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced);
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-868124227
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 35s | master passed |
| +1 :green_heart: | compile | 3m 40s | master passed |
| +1 :green_heart: | checkstyle | 1m 16s | master passed |
| +1 :green_heart: | spotbugs | 2m 31s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 35s | the patch passed |
| +1 :green_heart: | compile | 3m 37s | the patch passed |
| -0 :warning: | javac | 0m 31s | hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17) |
| -0 :warning: | checkstyle | 0m 14s | hbase-balancer: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0) |
| -0 :warning: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | hadoopcheck | 18m 7s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 50m 23s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 8f1c3a697430 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/whitespace-eol.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-872975020
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 11s | master passed |
| +1 :green_heart: | compile | 1m 35s | master passed |
| +1 :green_heart: | shadedjars | 8m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 13s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 6m 13s | hbase-balancer in the patch passed. |
| +1 :green_heart: | unit | 125m 20s | hbase-server in the patch passed. |
| | | 164m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f16fc0ff8a25 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5ef5e64010 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/testReport/ |
| Max. process+thread count | 3585 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r658053232
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +302,54 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0)
+ {
+ LOG.error("At least one cost function needs a multiplier > 0");
+ return false;
Review comment:
give a pointer in this message about what configs they should be looking for
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r668099448
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +303,55 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
Review comment:
yes.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -423,8 +424,9 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
maxSteps);
}
}
- LOG.info("start StochasticLoadBalancer.balancer, initCost=" + currentCost + ", functionCost="
- + functionCost() + " computedMaxSteps: " + computedMaxSteps);
+ LOG.info("Start StochasticLoadBalancer.balancer, initial weighted average imbalance={}, "
Review comment:
the functionCost has a ";" at the end so I removed the ",". The function cost explain the values. Here is the sample log from runs again which I posted above: functionCost=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced);
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-872977588
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 8s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 35s | master passed |
| +1 :green_heart: | compile | 1m 20s | master passed |
| +1 :green_heart: | shadedjars | 8m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 21s | the patch passed |
| +1 :green_heart: | javac | 1m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 6m 59s | hbase-balancer in the patch passed. |
| +1 :green_heart: | unit | 130m 45s | hbase-server in the patch passed. |
| | | 169m 9s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1593bbaeeb32 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 5ef5e64010 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/testReport/ |
| Max. process+thread count | 3898 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/8/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-857257398
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 29s | master passed |
| +1 :green_heart: | compile | 0m 21s | master passed |
| +1 :green_heart: | shadedjars | 8m 20s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 0m 22s | the patch passed |
| +1 :green_heart: | javac | 0m 22s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 15s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 5m 55s | hbase-balancer in the patch passed. |
| | | 34m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a9f044bb8069 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/testReport/ |
| Max. process+thread count | 318 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] saintstack commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646046394
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
Review comment:
Should be "Calculating plan. May take up to {} seconds to complete."
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
+ "Start calculating moving plan without logging info for up to {} secs",
+ isByTable ? String.format("table (%s)", tableName) : "cluster",
Review comment:
This string is a little confusing... We are balancing by table or by cluster? Should this fact be in the previous string?... "Calculating a plan for table=TABLENAME.... " or Calculating a plan for the full cluster..." Think of the poor operator reading the log message.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
Review comment:
We should name what we did here the same as what it was called in the start message. Here it is 'new load balance plan'. In the start log it is a 'moving plan'.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
Review comment:
Rather than 'move plan to be submitted...' just say 'Executing plan'.
Its called 'cost' but is that the right term? Is it our assessment of 'balance'?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
+ endTime - startTime, step, plans.size(), initCost, currentCost);
sendRegionPlansToRingBuffer(plans, currentCost, initCost, initFunctionTotalCosts, step);
return plans;
}
LOG.info("Could not find a better load balance plan. Tried {} different configurations in " +
Review comment:
Should be named same here as in start log.
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
Review comment:
What is cost? And multiplier? And how do the relate to whether a cluster is balanced or not?
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
If balanced, shouldn't it say why it thinks its balanced?
--
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
[GitHub] [hbase] busbey commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r658290872
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +302,54 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0)
+ {
+ LOG.error("At least one cost function needs a multiplier > 0");
+ return false;
Review comment:
I agree that we should not list all of the cost functions. How about if we just include a note like "the default cost functions should have configs named similar to hbase.master.balancer.stochastic.regionCountCost with _regionCount_ replaced with the specific cost function name."
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646895341
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
+ balanced ? "Skipping load balancing because balanced" :
Review comment:
https://issues.apache.org/jira/browse/HBASE-25666
--
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
[GitHub] [hbase] busbey commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
busbey commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-869030671
Looks like precommit is still flagging a few warnings new with the PR:
> javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt
> -- | --
> checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt
> whitespace | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/7/artifact/yetus-general-check/output/whitespace-eol.txt
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856944265
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 54s | master passed |
| +1 :green_heart: | compile | 0m 21s | master passed |
| +1 :green_heart: | shadedjars | 8m 17s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 18s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 16s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 3m 18s | hbase-balancer in the patch failed. |
| | | 30m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 542fc164681b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/testReport/ |
| Max. process+thread count | 354 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r658088094
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -303,54 +302,54 @@ private String getBalanceReason(double total, double sumMultiplier) {
boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
ClusterLoadState cs = new ClusterLoadState(cluster.clusterState);
if (cs.getNumServers() < MIN_SERVER_BALANCE) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Not running balancer because only " + cs.getNumServers()
+ LOG.info("Not running balancer because only " + cs.getNumServers()
+ " active regionserver(s)");
- }
sendRejectionReasonToRingBuffer(() -> "The number of RegionServers " + cs.getNumServers() +
" < MIN_SERVER_BALANCE(" + MIN_SERVER_BALANCE + ")", null);
return false;
}
if (areSomeRegionReplicasColocated(cluster)) {
+ LOG.info("Running balancer because at least one server hosts replicas of the same region.");
return true;
}
if (idleRegionServerExist(cluster)){
+ LOG.info("Running balancer because cluster has idle server(s).");
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
+ }
+ if (sumMultiplier <= 0)
+ {
+ LOG.error("At least one cost function needs a multiplier > 0");
+ return false;
Review comment:
I am afraid it would be too wordy. It is well documented, logged and used already. The enhanced logging logs enough about cost functions and their multipliers.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-867969037
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 20s | master passed |
| +1 :green_heart: | compile | 1m 33s | master passed |
| +1 :green_heart: | shadedjars | 8m 30s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 3s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 33s | the patch passed |
| +1 :green_heart: | compile | 1m 38s | the patch passed |
| +1 :green_heart: | javac | 1m 38s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 44s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 0s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 6m 5s | hbase-balancer in the patch passed. |
| -1 :x: | unit | 125m 9s | hbase-server in the patch failed. |
| | | 166m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ba9cb446b97d 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d11dc81721 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/testReport/ |
| Max. process+thread count | 3302 (vs. ulimit of 30000) |
| modules | C: hbase-balancer hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/6/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#issuecomment-856948709
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 0s | master passed |
| +1 :green_heart: | compile | 0m 35s | master passed |
| +1 :green_heart: | checkstyle | 0m 18s | master passed |
| +1 :green_heart: | spotbugs | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| -0 :warning: | checkstyle | 0m 15s | hbase-balancer: The patch generated 4 new + 0 unchanged - 0 fixed = 4 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 1s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 0m 45s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 37m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3356 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b15d350f443f 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / be14605898 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-balancer U: hbase-balancer |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3356/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646861474
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -343,15 +343,15 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
costFunctions);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ LOG.info("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
Review comment:
Hmm, right. Need to explain cost /multiplier is the measurement how balanced the cluster for balancer at a scale on [0,1] 0 being the perfectly balanced and 1 is the worst it could be. But the actual value still doesn't seem very intuitive to operator. We can explain if we demand the max of absolute number of unbalanced nodes to be constant, the bigger the cluster, the smaller of this value would be.
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r657546722
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -319,38 +318,33 @@ boolean needsBalance(TableName tableName, BalancerClusterState cluster) {
return true;
}
+ sumMultiplier = 0.0f;
double total = 0.0;
- float sumMultiplier = 0.0f;
for (CostFunction c : costFunctions) {
float multiplier = c.getMultiplier();
- if (multiplier <= 0) {
- LOG.trace("{} not needed because multiplier is <= 0", c.getClass().getSimpleName());
- continue;
- }
+ double cost = c.cost();
if (!c.isNeeded()) {
LOG.trace("{} not needed", c.getClass().getSimpleName());
continue;
}
+ total += cost * multiplier;
sumMultiplier += multiplier;
- total += c.cost() * multiplier;
}
- boolean balanced = total <= 0 || sumMultiplier <= 0 ||
- (sumMultiplier > 0 && (total / sumMultiplier) < minCostNeedBalance);
+ boolean balanced = (total / sumMultiplier < minCostNeedBalance);
+
if (balanced) {
final double calculatedTotal = total;
- final double calculatedMultiplier = sumMultiplier;
- sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal, calculatedMultiplier),
- costFunctions);
- }
- if (LOG.isDebugEnabled()) {
- LOG.debug("{} {}; total cost={}, sum multiplier={}; cost/multiplier to need a balance is {}",
- balanced ? "Skipping load balancing because balanced" : "We need to load balance",
- isByTable ? String.format("table (%s)", tableName) : "cluster",
- total, sumMultiplier, minCostNeedBalance);
- if (LOG.isTraceEnabled()) {
- LOG.trace("Balance decision detailed function costs={}", functionCost());
- }
+ sendRejectionReasonToRingBuffer(() -> getBalanceReason(calculatedTotal), costFunctions);
+ LOG.info("{} - skipping load balancing because weighted average imbalance={} > threshold({})."
+ + "functionCost={}."
+ + "If you want more aggressive balancing, either lower minCostNeedbalance {}"
+ + "or increase the relative weight(s) of the specific cost function(s).",
+ isByTable ? "Table specific ("+tableName+")" : "Cluster wide", functionCost(),
+ total / sumMultiplier, minCostNeedBalance);
Review comment:
Updated log from test run: Table specific (table1) - skipping load balancing because weighted average imbalance=RegionCountSkewCostFunction : (multiplier=500.0, imbalance=0.0, balanced); PrimaryRegionCountSkewCostFunction : (not needed); MoveCostFunction : (multiplier=7.0, imbalance=0.0, balanced); RackLocalityCostFunction : (multiplier=15.0, imbalance=0.0, balanced); TableSkewCostFunction : (multiplier=35.0, imbalance=1.0); RegionReplicaHostCostFunction : (not needed); RegionReplicaRackCostFunction : (not needed); ReadRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); CPRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); WriteRequestCostFunction : (multiplier=5.0, imbalance=0.0, balanced); MemStoreSizeCostFunction : (multiplier=5.0, imbalance=0.0, balanced); StoreFileCostFunction : (multiplier=5.0, imbalance=0.0, balanced); > threshold(0.06013745704467354).functionCost=1.0. If you want more aggressive balancing, either lower hbase.master.balancer.
stochastic.minCostNeedBalance from {}or increase the relative multiplier(s) of the specific cost function(s).
--
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
[GitHub] [hbase] clarax commented on a change in pull request #3356: HBASE-25973 Balancer should explain progress in a better way in log
Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #3356:
URL: https://github.com/apache/hbase/pull/3356#discussion_r646862825
##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -470,17 +470,17 @@ private long calculateMaxSteps(BalancerClusterState cluster) {
updateStochasticCosts(tableName, curOverallCost, curFunctionCosts);
if (initCost > currentCost) {
List<RegionPlan> plans = createRegionPlans(cluster);
- LOG.info("Finished computing new load balance plan. Computation took {}" +
- " to try {} different iterations. Found a solution that moves " +
- "{} regions; Going from a computed cost of {}" +
- " to a new cost of {}", java.time.Duration.ofMillis(endTime - startTime),
- step, plans.size(), initCost, currentCost);
+ LOG.info("Finished computing new load balance plan. Computation took {} ms" +
+ " to try {} different iterations. Found a solution that moves " +
+ "{} regions; Going from a computed cost of {}" +
+ " to a new cost of {}. Move plan to be submitted to master to execute",
Review comment:
Yes. @busbey after submitting the moves, balancer will rest for another time period before it resumes. If the moves are complete, it will look again to see if we need to move more. otherwise, it will skip.
--
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