J-HowHuang commented on code in PR #15891:
URL: https://github.com/apache/pinot/pull/15891#discussion_r2138406056
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceConfig.java:
##########
@@ -32,9 +32,9 @@ public class RebalanceConfig {
public static final long DEFAULT_EXTERNAL_VIEW_CHECK_INTERVAL_IN_MS = 1000L;
// 1 second
public static final long DEFAULT_EXTERNAL_VIEW_STABILIZATION_TIMEOUT_IN_MS =
3600000L; // 1 hour
- // Whether to rebalance table in dry-run mode
+ // Whether to rebalance table in dry-run mode.
@JsonProperty("dryRun")
- @ApiModelProperty(example = "false")
+ @ApiModelProperty(example = "true")
Review Comment:
Not sure when the UI will be released. Before that all tenant rebalance
users are expected to use this Swagger interface as the only way to interact
with this functionality.
With the same design concern to UI, operation should default to be a dry-run
and they should manually toggle it on to actually run the operation, to prevent
users from accidental triggers of impactful operation. The default value also
eases the users from playing with the API, where they don't need to worry about
forgetting to change dryRun to true everytime. Anyway I think it's nice to have
this default.
To make `dryRun` and `preChecks` a query params, I'll need to hide them from
the `rebalanceConfig` request body to avoid confusion. But there will be
another confusion in the code that why these are hidden in `rebalanceConfig`,
which is why I compromised to leave them in the body
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
