xiangfu0 commented on code in PR #18755:
URL: https://github.com/apache/pinot/pull/18755#discussion_r3409506098
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/admin/TenantAdminClient.java:
##########
@@ -317,34 +325,33 @@ public String cancelRebalance(String jobId)
return response.toString();
}
- /**
- * Rebalances all tables that are part of the tenant.
- *
- * @param tenantName Name of the tenant
- * @param degreeOfParallelism Number of table rebalance jobs allowed to run
at the same time
- * @param includeTableTypes Comma-separated list of table types to include
(optional)
- * @param excludeTableTypes Comma-separated list of table types to exclude
(optional)
- * @param rebalanceMode Rebalance mode (optional)
- * @return Rebalance result
- * @throws PinotAdminException If the request fails
- */
- public String rebalanceTenant(String tenantName, int degreeOfParallelism,
@Nullable String includeTableTypes,
- @Nullable String excludeTableTypes, @Nullable String rebalanceMode)
+ /// Rebalances all tables that are part of the tenant.
+ ///
+ /// The controller endpoint reads the include/exclude filters from the
`includeTables`/`excludeTables`
+ /// query params and requires a `TenantRebalanceConfig` request body; this
method sends an empty config object
+ /// so the query params can populate it. Use
[#rebalanceTenantWithConfig(String, String, Map)] when you need to
+ /// supply a full rebalance config.
+ ///
+ /// @param tenantName Name of the tenant
+ /// @param degreeOfParallelism Number of table rebalance jobs allowed to run
at the same time
+ /// @param includeTables Comma-separated list of tables (with type) to
include (optional)
+ /// @param excludeTables Comma-separated list of tables (with type) to
exclude (optional)
+ /// @return Rebalance result
+ /// @throws PinotAdminException If the request fails
+ public String rebalanceTenant(String tenantName, int degreeOfParallelism,
@Nullable String includeTables,
Review Comment:
This also deletes a published `pinot-java-client` overload in place. Even if
the old include/exclude/rebalance-mode params were wired incorrectly, removing
`rebalanceTenant(String, int, String, String, String)` will break existing
callers at compile/link time. Please leave a deprecated compatibility overload
(or explicit runtime validation) instead of dropping the method in one step.
##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/admin/TableAdminClient.java:
##########
@@ -378,26 +378,25 @@ public String validateTableConfig(String tableConfig)
return response.toString();
}
- /**
- * Rebalances a table (reassigns instances and segments).
- *
- * @param tableName Name of the table to rebalance
- * @param noDowntime Whether to allow rebalance without downtime
- * @param rebalanceMode Rebalance mode (default or specific)
- * @param minReplicasToKeepAfterRebalance Minimum replicas to keep after
rebalance
- * @return Rebalance result
- * @throws PinotAdminException If the request fails
- */
- public String rebalanceTable(String tableName, boolean noDowntime, @Nullable
String rebalanceMode,
+ /// Rebalances a table (reassigns instances and segments).
+ ///
+ /// Maps to the controller's `downtime` and `minAvailableReplicas` query
params (see
+ /// [#rebalanceTable(String, String, boolean, boolean, boolean, boolean,
int)] for the full parameter set, or
+ /// [#rebalanceTable(String, Map)] to pass arbitrary params).
+ ///
+ /// @param tableName Name of the table to rebalance
+ /// @param noDowntime Whether the rebalance should avoid downtime (maps to
`downtime=false`)
+ /// @param minReplicasToKeepAfterRebalance Minimum number of replicas to
keep available during a no-downtime
+ /// rebalance (maps to
`minAvailableReplicas`)
+ /// @return Rebalance result
+ /// @throws PinotAdminException If the request fails
+ public String rebalanceTable(String tableName, boolean noDowntime,
Review Comment:
This removes the existing 4-arg `rebalanceTable(...)` overload from
`pinot-java-client`. That is a source/binary compatibility break for current
client consumers on upgrade; the README still documents
`rebalanceTable("myTable", true, "default", 1)`. Please keep the old signature
as a deprecated delegating shim while fixing the controller param mapping
underneath.
--
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]
