bbeaudreault commented on a change in pull request #3536:
URL: https://github.com/apache/hbase/pull/3536#discussion_r679500129
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
##########
@@ -1253,6 +1253,15 @@ default boolean balancer() throws IOException {
*/
boolean balance() throws IOException;
+ /**
+ * Invoke the balancer in dry run mode. Will show plan but not actually move
any regions.
+ * Can NOT run for various reasons. Check logs.
+ *
+ * @return <code>true</code> if dry run ran, <code>false</code> otherwise.
+ * @throws IOException if a remote or network exception occurs
+ */
+ boolean dryRunBalance() throws IOException;
Review comment:
Ok, so it's more acceptable to modify the public API in this case? Here
are some possibilities I can come up with:
- Modify the existing `balance(boolean force)` to add a new `boolean dryRun`
argument. So `balance(boolean force, boolean dryRun)`. I don't love putting 2
booleans back to back like this, especially since force + boolean doesn't quite
make sense.
- In this case I think I'd actually have to add a new method to preserve
backwards compatibility, and the the existing `balance(boolean force)` would
delegate to `return balance(force, false)`.
- Change the going forward signature to a RunMode enum (or other name), with
`REQUEST`, `FORCE`, `DRY_RUN`.
- In this case I would need a new `balance(RunMode runMode)`, and then the
existing balance would delegate to `return balance(force ? RunMode.FORCE :
RunMode.REQUEST)`
- Change the going forward signature to a BalancerConfig object. We have a
lot of options for how the BalancerConfig is constructed, either as a builder
or factory methods.
- Similar to above, I'd add a new `balance(BalancerConfig)`, and delegate
based on the value of the force boolean.
I honestly think the last option is the best for long term as it sets us up
for future expansion, but it's also the most complicated. I'd be fine to add
that, but all of these options change the public API. I'm not sure which is the
lesser of all evils from that perspective.
--
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]
