J-HowHuang commented on code in PR #15891:
URL: https://github.com/apache/pinot/pull/15891#discussion_r2136690876
##########
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:
It will cause issue indeed. But consider that this should be deprecated in
the near future, and the ease to remove the old algo, this introduce the least
code change.
I can add type check to make it safe here (e.g. throw exception if the impl
is not DefaultRebalancer), wdyt?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
