vaijosh commented on code in PR #7932:
URL: https://github.com/apache/hbase/pull/7932#discussion_r2930776399
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/CacheAwareLoadBalancer.java:
##########
@@ -193,10 +253,12 @@ public void throttle(RegionPlan plan) {
+ "Throttling move for {}ms.",
plan.getRegionInfo().getEncodedName(), plan.getDestination(),
sleepTime);
}
- try {
- Thread.sleep(sleepTime);
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
+ synchronized (this) {
+ try {
+ wait(sleepTime);
Review Comment:
Comment line here to explain its giving up monitor.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/CacheAwareLoadBalancer.java:
##########
@@ -79,6 +107,38 @@ public synchronized void loadConf(Configuration
configuration) {
sleepTime = configuration.getLong(MOVE_THROTTLING,
MOVE_THROTTLING_DEFAULT.toMillis());
}
+ /**
+ * Sets {@link #isBalancing} to {@code true} before a balance run starts.
+ */
+ @Override
+ public void onBalancingStart() {
+ LOG.debug("setting isBalancing to true as balance is starting");
Review Comment:
nit: Please keep first character of sentence capital.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -2036,80 +2036,86 @@ public BalanceResponse balance(BalanceRequest request)
throws IOException {
}
synchronized (this.balancer) {
- // Only allow one balance run at at time.
- if (this.assignmentManager.getRegionTransitScheduledCount() > 0) {
- List<RegionStateNode> regionsInTransition =
assignmentManager.getRegionsInTransition();
- // if hbase:meta region is in transition, result of assignment cannot
be recorded
- // ignore the force flag in that case
- boolean metaInTransition =
assignmentManager.isMetaRegionInTransition();
- List<RegionStateNode> toPrint = regionsInTransition;
- int max = 5;
- boolean truncated = false;
- if (regionsInTransition.size() > max) {
- toPrint = regionsInTransition.subList(0, max);
- truncated = true;
- }
+ try {
+ this.balancer.onBalancingStart();
+
+ // Only allow one balance run at at time.
+ if (this.assignmentManager.getRegionTransitScheduledCount() > 0) {
+ List<RegionStateNode> regionsInTransition =
assignmentManager.getRegionsInTransition();
+ // if hbase:meta region is in transition, result of assignment
cannot be recorded
+ // ignore the force flag in that case
+ boolean metaInTransition =
assignmentManager.isMetaRegionInTransition();
+ List<RegionStateNode> toPrint = regionsInTransition;
+ int max = 5;
+ boolean truncated = false;
+ if (regionsInTransition.size() > max) {
+ toPrint = regionsInTransition.subList(0, max);
+ truncated = true;
+ }
- if (!request.isIgnoreRegionsInTransition() || metaInTransition) {
- LOG.info("Not running balancer (ignoreRIT=false" + ", metaRIT=" +
metaInTransition
- + ") because " + assignmentManager.getRegionTransitScheduledCount()
- + " region(s) are scheduled to transit " + toPrint
- + (truncated ? "(truncated list)" : ""));
+ if (!request.isIgnoreRegionsInTransition() || metaInTransition) {
+ LOG.info("Not running balancer (ignoreRIT=false" + ", metaRIT=" +
metaInTransition
+ + ") because " +
assignmentManager.getRegionTransitScheduledCount()
+ + " region(s) are scheduled to transit " + toPrint
+ + (truncated ? "(truncated list)" : ""));
+ return responseBuilder.build();
+ }
+ }
+ if (this.serverManager.areDeadServersInProgress()) {
+ LOG.info("Not running balancer because processing dead
regionserver(s): "
+ + this.serverManager.getDeadServers());
return responseBuilder.build();
}
- }
- if (this.serverManager.areDeadServersInProgress()) {
- LOG.info("Not running balancer because processing dead
regionserver(s): "
- + this.serverManager.getDeadServers());
- return responseBuilder.build();
- }
- if (this.cpHost != null) {
- try {
- if (this.cpHost.preBalance(request)) {
- LOG.debug("Coprocessor bypassing balancer request");
+ if (this.cpHost != null) {
+ try {
+ if (this.cpHost.preBalance(request)) {
+ LOG.debug("Coprocessor bypassing balancer request");
+ return responseBuilder.build();
+ }
+ } catch (IOException ioe) {
+ LOG.error("Error invoking master coprocessor preBalance()", ioe);
return responseBuilder.build();
}
- } catch (IOException ioe) {
- LOG.error("Error invoking master coprocessor preBalance()", ioe);
- return responseBuilder.build();
}
- }
- Map<TableName, Map<ServerName, List<RegionInfo>>> assignments =
-
this.assignmentManager.getRegionStates().getAssignmentsForBalancer(tableStateManager,
- this.serverManager.getOnlineServersList());
- for (Map<ServerName, List<RegionInfo>> serverMap : assignments.values())
{
-
serverMap.keySet().removeAll(this.serverManager.getDrainingServersList());
- }
+ Map<TableName, Map<ServerName, List<RegionInfo>>> assignments =
Review Comment:
I can see space character here in diff? Did you fix formatting here and next
few lines?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/CacheAwareLoadBalancer.java:
##########
@@ -79,6 +107,38 @@ public synchronized void loadConf(Configuration
configuration) {
sleepTime = configuration.getLong(MOVE_THROTTLING,
MOVE_THROTTLING_DEFAULT.toMillis());
}
+ /**
+ * Sets {@link #isBalancing} to {@code true} before a balance run starts.
+ */
+ @Override
+ public void onBalancingStart() {
+ LOG.debug("setting isBalancing to true as balance is starting");
+ isBalancing.set(true);
+ }
+
+ /**
+ * Sets {@link #isBalancing} to {@code false} after a balance run completes
and applies any
+ * pending configuration that arrived during balancing.
+ */
+ @Override
+ public void onBalancingComplete() {
+ LOG.debug("setting isBalancing to false as balance is completed");
Review Comment:
nit: Please keep first character of sentence capital.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
