bbeaudreault commented on a change in pull request #3536:
URL: https://github.com/apache/hbase/pull/3536#discussion_r686203022



##########
File path: 
hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
##########
@@ -541,6 +544,11 @@ public boolean balanceRSGroup(String groupName) throws 
IOException {
           getRSGroupAssignmentsByTable(master.getTableStateManager(), 
groupName);
       List<RegionPlan> plans = balancer.balanceCluster(assignmentsByTable);
       boolean balancerRan = !plans.isEmpty();
+
+      if (request.isDryRun()) {
+        return balancerRan;

Review comment:
       Ah ok. I'll leave this as-is for now. Thanks for the discussion!
   
   I think we're in agreement on the value of "some non-bool return value" 
here. My only concern with leaving the boolean return value on my new `boolean 
balance(BalanceRequest request) throws IOException` (and async counterpart) 
method is if we'd have trouble modifying that interface in a follow-up PR. Any 
change to that method's return value will be a breaking change, so if we want 
to do that without further deprecations/gymnastics then we should at least put 
a placeholder now. I guess it would be fine as long as the follow-up comes 
before 2.5.0, right?




-- 
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]


Reply via email to