J-HowHuang commented on code in PR #15891:
URL: https://github.com/apache/pinot/pull/15891#discussion_r2136712870
##########
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:
It uses this value as an example in the Swagger UI, and if you click that
"try it out" button it turns into the auto-filled in the value, as you can see
here in the body part.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java:
##########
@@ -690,9 +686,46 @@ public SuccessResponse deleteTenant(
@ApiOperation(value = "Rebalances all the tables that are part of the
tenant")
public TenantRebalanceResult rebalance(
@ApiParam(value = "Name of the tenant whose table are to be rebalanced",
required = true)
- @PathParam("tenantName") String tenantName, @ApiParam(required = true)
TenantRebalanceConfig config) {
+ @PathParam("tenantName") String tenantName,
+ @ApiParam(value = "Number of table rebalance jobs allowed to run at the
same time", required = true, example =
+ "1")
+ @QueryParam("degreeOfParallelism") Integer degreeOfParallelism,
+ @ApiParam(value =
+ "Comma separated list of tables (with OFFLINE or REALTIME suffix)
that are allowed in this tenant rebalance"
+ + " job. Leave blank to allow all tables from the tenant",
example = "")
+ @QueryParam("allowTables") String allowTables,
+ @ApiParam(value =
+ "Comma separated list of tables (with OFFLINE or REALTIME suffix)
that are blocked in this tenant rebalance"
+ + " job. These table will be removed from allowTables", example
= "")
+ @QueryParam("blockTables") String blockTables,
+ @ApiParam(value = "Show full rebalance results of each table in the
response", example = "false")
+ @QueryParam("verboseResult") Boolean verboseResult,
+ @ApiParam(name = "rebalanceConfig", value = "The rebalance config
applied to run every table", required = true)
+ TenantRebalanceConfig config) {
// TODO decide on if the tenant rebalance should be database aware or not
config.setTenantName(tenantName);
+ // Query params should override the config provided in the body, if present
+ if (degreeOfParallelism != null) {
+ config.setDegreeOfParallelism(degreeOfParallelism);
+ }
+ if (verboseResult != null) {
+ config.setVerboseResult(verboseResult);
+ }
+ if (allowTables != null) {
+ config.setAllowTables(Arrays.stream(StringUtil.split(allowTables, ',',
0)).map(String::strip).collect(
+ Collectors.toSet()));
+ }
+ if (blockTables != null) {
+ config.setBlockTables(Arrays.stream(StringUtil.split(blockTables, ',',
0)).map(String::strip).collect(
+ Collectors.toSet()));
+ }
+ if (!config.getParallelWhitelist().isEmpty() ||
!config.getParallelBlacklist().isEmpty()) {
+ // If the parallel whitelist or blacklist is set, the old tenant
rebalance logic will be used
+ // TODO: Deprecate the support for this in the future
+ LOGGER.warn("Using the old tenant rebalance logic because parallel
whitelist or blacklist is set. "
+ + "This will be deprecated in the future.");
+ return ((DefaultTenantRebalancer)
_tenantRebalancer).rebalanceWithParallelAndSequential(config);
Review Comment:
Sure it's a fair point. I can make this change
--
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]
