Jackie-Jiang commented on a change in pull request #7556:
URL: https://github.com/apache/pinot/pull/7556#discussion_r738958354
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java
##########
@@ -113,4 +114,27 @@ public String getTableInstances(
ret.set("server", servers); // Keeping compatibility with previous API,
so "server" and "brokers"
return ret.toString();
}
+
+ @GET
+ @Path("/tables/{tableNameWithType}/livebrokers")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "List the brokers serving a table", notes = "List live
brokers of the given table based on EV")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 404, message = "Table not found"),
+ @ApiResponse(code = 500, message = "Internal server error")})
+ public String getLiveBrokersForTable(
+ @ApiParam(value = "Table name with type", required = true)
+ @PathParam("tableNameWithType") String tableNameWithType) {
+ ObjectNode ret = JsonUtils.newObjectNode();
+ ret.put("tableName", tableNameWithType);
+ List<String> liveBrokersForTable =
_pinotHelixResourceManager.getLiveBrokersForTable(tableNameWithType);
Review comment:
Let's directly `return
_pinotHelixResourceManager.getLiveBrokersForTable(tableNameWithType);` instead
of constructing a json object. User should expect a list per the API definition.
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String
tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
Review comment:
(minor) Use javadoc comment block
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String
tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
+ // Each entry in the broker list is of the following format:
+ // Broker_hostname_port
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
Review comment:
(optional) Suggest making the input `tableName` (table name with or
without type). If the input is raw table name, search for both `OFFLINE` and
`REALTIME` table live brokers and intersect them.
This can be done in a separate PR.
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2862,6 +2862,25 @@ public TableStats getTableStats(String
tableNameWithType) {
return new TableStats(creationTime);
}
+ // Return the list of live brokers serving the corresponding table.
+ // Each entry in the broker list is of the following format:
+ // Broker_hostname_port
+ public List<String> getLiveBrokersForTable(String tableNameWithType) {
+ ExternalView ev =
_helixDataAccessor.getProperty(_keyBuilder.externalView(Helix.BROKER_RESOURCE_INSTANCE));
+ if (ev == null) {
+ return Collections.EMPTY_LIST;
+ }
+ Map<String, String> brokerToStateMap = ev.getStateMap(tableNameWithType);
+ List<String> hosts = new ArrayList<>();
+ if (brokerToStateMap != null) {
+ for (Map.Entry<String, String> entry : brokerToStateMap.entrySet()) {
+ if ("ONLINE".equalsIgnoreCase(entry.getValue())) {
+ hosts.add(entry.getKey());
+ }
+ }
+ }
+ return hosts;
+ }
Review comment:
(minor) Add an empty line following the method
--
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]
