Jackie-Jiang commented on code in PR #9662:
URL: https://github.com/apache/pinot/pull/9662#discussion_r1028434895
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -687,6 +688,26 @@ public RebalanceResult rebalance(
}
}
+ @GET
+ @Path("/tables/{tableName}/rebalance/status")
Review Comment:
Let's make the API more specific, and it might be better to put it under the
segment rest API: `/segments/{tableName}/externalViewMismatch` and returns a
map from the mismatch segment to its ideal state and external view
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -687,6 +688,26 @@ public RebalanceResult rebalance(
}
}
+ @GET
+ @Path("/tables/{tableName}/rebalance/status")
+ @ApiOperation(value = "Table rebalance status", notes = "Get segments
without the same state in EV and IS ")
+ public String getTableRebalanceStatus(
+ @ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME", required = true)
@QueryParam("type") String tableTypeStr) {
+ try {
+ String tableNameWithType = constructTableNameWithType(tableName,
tableTypeStr);
Review Comment:
(code format) Please reformat the changes with [Pinot
Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#setup-ide)
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3082,6 +3083,17 @@ public RebalanceResult rebalanceTable(String
tableNameWithType, Configuration re
return new TableRebalancer(_helixZkManager).rebalance(tableConfig,
rebalanceConfig);
}
+ public Map<String, MapDifference.ValueDifference<Map<String, String>>>
rebalanceTableStatus(String tableNameWithType)
+ throws TableNotFoundException {
+ TableConfig tableConfig = getTableConfig(tableNameWithType);
Review Comment:
No need to read the table config. We should check both IS and EV exist
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -733,6 +754,7 @@ private String constructTableNameWithType(String tableName,
String tableTypeStr)
try {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
} catch (Exception e) {
+
Review Comment:
(nit) Please revert some unnecessary empty line changes in this PR
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -383,6 +385,25 @@ public RebalanceResult rebalance(TableConfig tableConfig,
Configuration rebalanc
}
}
+ public Map<String, MapDifference.ValueDifference<Map<String, String>>>
rebalanceStatus(String tableNameWithType,
Review Comment:
Don't put this into the `TableRebalancer`. We can directly have it in
`PinotHelixResourceManager`
--
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]
