bbeaudreault commented on a change in pull request #3536:
URL: https://github.com/apache/hbase/pull/3536#discussion_r679474465
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1809,6 +1809,10 @@ public boolean balance(boolean force) throws IOException
{
List<RegionPlan> plans = this.balancer.balanceCluster(assignments);
+ if (runMode.isDryRun()) {
+ return true;
+ }
+
Review comment:
@joshelser thanks for the review! I wanted to point out that a few lines
below this diff (line 1825) is the `postBalance` coprocessor call. We skip that
for dry run mode, but a few lines above this diff (line 1790) is the
`preBalance` coprocessor call, which we _do_ call.
I chose to call pre and not post because it seems like maybe someone might
use `preBalance` to check or set extra state that might be necessary for the
balancer, while I was concerned that `postBalance` might make decisions based
on balance plans that are passed in but were not actually executed.
I've been thinking maybe we should just skip both or call both, and
wondering if you have any thoughts. We could also pass in the RunMode into the
coprocessor, but then we get into coprocessor interface changes. Not sure
that's worth it?
--
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-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
