pkuwm commented on a change in pull request #1416:
URL: https://github.com/apache/helix/pull/1416#discussion_r500102221
##########
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
##########
@@ -442,6 +442,50 @@ public Response getClusterTopology(@PathParam("clusterId")
String clusterId) thr
return OK(objectMapper.writeValueAsString(clusterTopology));
}
+ @ResponseMetered(name = HttpConstants.READ_REQUEST)
+ @Timed(name = HttpConstants.READ_REQUEST)
+ @GET
+ @Path("{clusterId}/topologymap")
+ public Response getClusterTopologyMap(@PathParam("clusterId") String
clusterId) {
+ if (!doesClusterExist(clusterId)) {
+ return notFound(String.format("Cluster %s does not exist", clusterId));
+ }
+ HelixAdmin admin = getHelixAdmin();
+ Map<String, List<String>> topologyMap;
+ try {
+ topologyMap = admin.getClusterTopology(clusterId).getTopologyMap();
+ } catch (HelixException ex) {
+ return badRequest(ex.getMessage());
+ } catch (Exception ex) {
+ LOG.error("Failed to get cluster topology map fro cluster {}. Exception:
" + "{}.", clusterId,
+ ex);
+ return serverError(ex);
+ }
+ return JSONRepresentation(topologyMap);
+ }
+
+ @ResponseMetered(name = HttpConstants.READ_REQUEST)
+ @Timed(name = HttpConstants.READ_REQUEST)
+ @GET
+ @Path("{clusterId}/faultzonemap")
+ public Response getClusterFaultZoneMap(@PathParam("clusterId") String
clusterId) {
+ if (!doesClusterExist(clusterId)) {
+ return notFound(String.format("Cluster %s does not exist", clusterId));
+ }
+ HelixAdmin admin = getHelixAdmin();
+ Map<String, List<String>> faultZoneMap;
+ try {
+ faultZoneMap = admin.getClusterTopology(clusterId).getFaultZoneMap();
+ } catch (HelixException ex) {
+ return badRequest(ex.getMessage());
+ } catch (Exception ex) {
+ LOG.error("Failed to get cluster fault zone map fro cluster {}.
Exception: " + "{}.",
Review comment:
Typo: **fro** -> **from**
`LOG.error("Failed to get cluster fault zone map fro cluster {}.",
clusterId, ex);` to print out the stack trace?
Same in `getClusterTopologyMap `.
##########
File path:
helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
##########
@@ -442,6 +442,29 @@ public Response getClusterTopology(@PathParam("clusterId")
String clusterId) thr
return OK(objectMapper.writeValueAsString(clusterTopology));
}
+ @ResponseMetered(name = HttpConstants.READ_REQUEST)
+ @Timed(name = HttpConstants.READ_REQUEST)
+ @GET
+ @Path("{clusterId}/topologymap")
+ public Response getClusterTopologyMap(@PathParam("clusterId") String
clusterId) {
+ if (!doesClusterExist(clusterId)) {
Review comment:
I realized that this call is not a lightweight call. It has many calls
to zk to check all required below paths exist. I think this is pretty heavy.
And even the cluster exists at the usual case, this check makes the rest api
call pretty slow to respond, which I don't think is expected, right? And
actually `isClusterSetup` is called in `getClusterTopology` ->
`getClusterConfig()` -> `getConfigZnRecord` -- multiple duplicate calls to zk
just to check `isClusterSetup`. So the following `HelixException` catch is
good enough. No need to do the pre-check `if (!doesClusterExist(clusterId)) {`
```
requiredPaths.add(PropertyPathBuilder.idealState(clusterName));
requiredPaths.add(PropertyPathBuilder.clusterConfig(clusterName));
requiredPaths.add(PropertyPathBuilder.instanceConfig(clusterName));
requiredPaths.add(PropertyPathBuilder.resourceConfig(clusterName));
requiredPaths.add(PropertyPathBuilder.propertyStore(clusterName));
requiredPaths.add(PropertyPathBuilder.liveInstance(clusterName));
requiredPaths.add(PropertyPathBuilder.instance(clusterName));
requiredPaths.add(PropertyPathBuilder.externalView(clusterName));
requiredPaths.add(PropertyPathBuilder.controller(clusterName));
requiredPaths.add(PropertyPathBuilder.stateModelDef(clusterName));
requiredPaths.add(PropertyPathBuilder.controllerMessage(clusterName));
requiredPaths.add(PropertyPathBuilder.controllerError(clusterName));
requiredPaths.add(PropertyPathBuilder.controllerStatusUpdate(clusterName));
requiredPaths.add(PropertyPathBuilder.controllerHistory(clusterName));
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
