Jackie-Jiang commented on code in PR #10112:
URL: https://github.com/apache/pinot/pull/10112#discussion_r1067518405
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -621,27 +621,31 @@ private ObjectNode validateConfig(TableConfig
tableConfig, Schema schema, @Nulla
@Produces(MediaType.APPLICATION_JSON)
@Authenticate(AccessType.UPDATE)
@Path("/tables/{tableName}/rebalance")
- @ApiOperation(value = "Rebalances a table (reassign instances and segments
for a table)",
- notes = "Rebalances a table (reassign instances and segments for a
table)")
+ @ApiOperation(value = "Rebalances a table (reassign instances and segments
for a table)", notes = "Rebalances a "
+ + "table (reassign instances and segments for a table)")
public RebalanceResult rebalance(
@ApiParam(value = "Name of the table to rebalance", required = true)
@PathParam("tableName") String tableName,
@ApiParam(value = "OFFLINE|REALTIME", required = true)
@QueryParam("type") String tableTypeStr,
@ApiParam(value = "Whether to rebalance table in dry-run mode")
@DefaultValue("false") @QueryParam("dryRun")
- boolean dryRun,
+ boolean dryRun,
@ApiParam(value = "Whether to reassign instances before reassigning
segments") @DefaultValue("false")
@QueryParam("reassignInstances") boolean reassignInstances,
@ApiParam(value = "Whether to reassign CONSUMING segments for real-time
table") @DefaultValue("false")
- @QueryParam("includeConsuming") boolean includeConsuming, @ApiParam(
- value = "Whether to rebalance table in bootstrap mode (regardless of
minimum segment movement, reassign all "
+ @QueryParam("includeConsuming") boolean includeConsuming,
@ApiParam(value =
+ "Whether to rebalance table in bootstrap mode (regardless of minimum
segment movement, reassign all "
+ "segments in a round-robin fashion as if adding new segments to an
empty table)") @DefaultValue("false")
@QueryParam("bootstrap") boolean bootstrap,
@ApiParam(value = "Whether to allow downtime for the rebalance")
@DefaultValue("false") @QueryParam("downtime")
- boolean downtime, @ApiParam(
- value = "For no-downtime rebalance, minimum number of replicas to keep
alive during rebalance, or maximum "
+ boolean downtime, @ApiParam(value =
+ "For no-downtime rebalance, minimum number of replicas to keep alive
during rebalance, or maximum "
+ "number of replicas allowed to be unavailable if value is
negative") @DefaultValue("1")
- @QueryParam("minAvailableReplicas") int minAvailableReplicas, @ApiParam(
- value = "Whether to use best-efforts to rebalance (not fail the
rebalance when the no-downtime contract cannot "
- + "be achieved)") @DefaultValue("false") @QueryParam("bestEfforts")
boolean bestEfforts) {
+ @QueryParam("minAvailableReplicas") int minAvailableReplicas,
@ApiParam(value =
+ "Whether to use best-efforts to rebalance (not fail the rebalance when
the no-downtime contract cannot "
+ + "be achieved)") @DefaultValue("false") @QueryParam("bestEfforts")
boolean bestEfforts,
+ @ApiParam(value = "How often to check if external view converges with
ideal states") @DefaultValue("1000")
Review Comment:
Can you double check the format change? I feel it is not very readable
##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/RebalanceTableCommand.java:
##########
@@ -77,6 +77,14 @@ public class RebalanceTableCommand extends
AbstractBaseAdminCommand implements C
+ " cannot be achieved, false by default)")
private boolean _bestEfforts = false;
+ @CommandLine.Option(names = {"-externalViewCheckIntervalMs"},
+ description = "How often to check if external view converges with ideal
view")
+ private long _externalViewCheckIntervalMs = 1000;
Review Comment:
Should we use the constant introduced in `RebalanceConfigConstants` as
default?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -912,6 +916,14 @@ public boolean enableSegmentRelocatorLocalTierMigration() {
return
getProperty(ControllerPeriodicTasksConf.SEGMENT_RELOCATOR_ENABLE_LOCAL_TIER_MIGRATION,
false);
}
+ public long getSegmentRelocatorExternalViewCheckIntervalMs() {
+ return
getProperty(ControllerPeriodicTasksConf.SEGMENT_RELOCATOR_EXTERNAL_VIEW_CHECK_INTERVAL_MS,
1000);
Review Comment:
Should we use the constant introduced in `RebalanceConfigConstants` as
default?
--
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
