bbeaudreault commented on a change in pull request #3536:
URL: https://github.com/apache/hbase/pull/3536#discussion_r686070468
##########
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:
Ok so my initial thought here was to agree with you about the richness
of the boolean, and to say that I didn't want to further mess with the
interface to change the return value away from boolean. But I was just
thinking, I'm already adding a new method that takes a BalanceRequest, so I
could add a BalanceResponse. We can maintain backwards compatibility by making
the default `balance()` and `balance(force)` methods call
`response.isBalancerRan()` or something:
```java
default balance() throws IOException {
return balance(BalanceRequest.defaultInstance()).isBalancerRan();
```
Something like that.
I've addressed most of your feedback in one commit already. I'll try to take
a stab at this now.
--
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]
