You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2021/12/01 19:58:03 UTC

[GitHub] [helix] junkaixue commented on a change in pull request #1917: Add rest API for take/free instance

junkaixue commented on a change in pull request #1917:
URL: https://github.com/apache/helix/pull/1917#discussion_r760513774



##########
File path: helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
##########
@@ -423,6 +422,23 @@ private StoppableCheck performCustomInstanceCheck(String clusterId, String insta
     return result;
   }
 
+  public static List<String> getListFromJsonPayload(JsonNode jsonContent) throws IOException {
+    if (jsonContent == null) {
+      return Collections.emptyList();
+    }
+    return OBJECT_MAPPER.convertValue(jsonContent, List.class);
+  }
+
+  public static Map<String, String> getMapFromJsonPayload(JsonNode jsonContent) throws IOException {
+    Map<String, String> result = new HashMap<>();
+    if (jsonContent == null) {
+      return result;
+    }
+    result = OBJECT_MAPPER.convertValue(jsonContent, new TypeReference<Map<String, String>>() {

Review comment:
       Would this throw exception?

##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java
##########
@@ -185,6 +200,137 @@ public Response isInstanceStoppable(
     return OK(OBJECT_MAPPER.writeValueAsString(stoppableCheck));
   }
 
+  /**
+   * Performs health checks, user designed operation check and execution for take an instance.
+   *
+   * @param jsonContent json payload
+   * @param clusterId cluster id
+   * @param instanceName Instance name to be checked
+   * @return json response representing if queried instance is stoppable
+   * @throws IOException if there is any IO/network error
+   */
+  @ResponseMetered(name = HttpConstants.WRITE_REQUEST)
+  @Timed(name = HttpConstants.WRITE_REQUEST)
+  @POST
+  @Path("takeInstance")
+  @Consumes(MediaType.APPLICATION_JSON)
+  public Response takeSingleInstance(
+      String jsonContent,
+      @PathParam("clusterId") String clusterId,
+      @PathParam("instanceName") String instanceName){
+
+    try {
+      JsonNode node = null;
+      if (jsonContent.length() != 0) {
+        node = OBJECT_MAPPER.readTree(jsonContent);
+      }
+      if (node == null) {
+        return badRequest("Invalid input for content : " + jsonContent);
+      }
+      MaintenanceOpInputFields inputFields = readMaintenanceInputFromJson(node);
+
+      MaintenanceManagementService maintenanceManagementService =
+          new MaintenanceManagementService((ZKHelixDataAccessor) getDataAccssor(clusterId),
+              getConfigAccessor(), inputFields.skipZKRead, inputFields.continueOnFailures,
+              getNamespace());
+
+      return JSONRepresentation(maintenanceManagementService
+          .takeInstance(clusterId, instanceName, inputFields.healthChecks,
+              inputFields.healthCheckConfig,
+              inputFields.operations,
+              inputFields.operationConfig, true));
+    } catch (Exception e) {
+      LOG.error("Failed to takeInstances:", e);
+      return badRequest("Failed to takeInstances: " + e.getMessage());
+    }
+  }
+
+  /**
+   * Performs health checks, user designed operation check and execution for free an instance.
+   *
+   * @param jsonContent json payload
+   * @param clusterId cluster id
+   * @param instanceName Instance name to be checked
+   * @return json response representing if queried instance is stoppable
+   * @throws IOException if there is any IO/network error
+   */
+  @ResponseMetered(name = HttpConstants.WRITE_REQUEST)
+  @Timed(name = HttpConstants.WRITE_REQUEST)
+  @POST
+  @Path("freeInstance")
+  @Consumes(MediaType.APPLICATION_JSON)
+  public Response freeSingleInstance(
+      String jsonContent,
+      @PathParam("clusterId") String clusterId,
+      @PathParam("instanceName") String instanceName){
+
+    try {
+      JsonNode node = null;
+      if (jsonContent.length() != 0) {
+        node = OBJECT_MAPPER.readTree(jsonContent);
+      }
+      if (node == null) {
+        return badRequest("Invalid input for content : " + jsonContent);
+      }
+      MaintenanceOpInputFields inputFields = readMaintenanceInputFromJson(node);

Review comment:
       This is dup code with previous function. Let's wrap it into readMaintenanceInputFromJson function?

##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/PerInstanceAccessor.java
##########
@@ -185,6 +200,137 @@ public Response isInstanceStoppable(
     return OK(OBJECT_MAPPER.writeValueAsString(stoppableCheck));
   }
 
+  /**
+   * Performs health checks, user designed operation check and execution for take an instance.
+   *
+   * @param jsonContent json payload
+   * @param clusterId cluster id
+   * @param instanceName Instance name to be checked
+   * @return json response representing if queried instance is stoppable
+   * @throws IOException if there is any IO/network error
+   */
+  @ResponseMetered(name = HttpConstants.WRITE_REQUEST)
+  @Timed(name = HttpConstants.WRITE_REQUEST)
+  @POST
+  @Path("takeInstance")
+  @Consumes(MediaType.APPLICATION_JSON)
+  public Response takeSingleInstance(
+      String jsonContent,
+      @PathParam("clusterId") String clusterId,
+      @PathParam("instanceName") String instanceName){
+
+    try {
+      JsonNode node = null;
+      if (jsonContent.length() != 0) {
+        node = OBJECT_MAPPER.readTree(jsonContent);
+      }
+      if (node == null) {
+        return badRequest("Invalid input for content : " + jsonContent);
+      }
+      MaintenanceOpInputFields inputFields = readMaintenanceInputFromJson(node);
+
+      MaintenanceManagementService maintenanceManagementService =
+          new MaintenanceManagementService((ZKHelixDataAccessor) getDataAccssor(clusterId),
+              getConfigAccessor(), inputFields.skipZKRead, inputFields.continueOnFailures,
+              getNamespace());
+
+      return JSONRepresentation(maintenanceManagementService
+          .takeInstance(clusterId, instanceName, inputFields.healthChecks,
+              inputFields.healthCheckConfig,
+              inputFields.operations,
+              inputFields.operationConfig, true));
+    } catch (Exception e) {
+      LOG.error("Failed to takeInstances:", e);
+      return badRequest("Failed to takeInstances: " + e.getMessage());
+    }
+  }
+
+  /**
+   * Performs health checks, user designed operation check and execution for free an instance.
+   *
+   * @param jsonContent json payload
+   * @param clusterId cluster id
+   * @param instanceName Instance name to be checked
+   * @return json response representing if queried instance is stoppable
+   * @throws IOException if there is any IO/network error
+   */
+  @ResponseMetered(name = HttpConstants.WRITE_REQUEST)
+  @Timed(name = HttpConstants.WRITE_REQUEST)
+  @POST
+  @Path("freeInstance")
+  @Consumes(MediaType.APPLICATION_JSON)
+  public Response freeSingleInstance(
+      String jsonContent,
+      @PathParam("clusterId") String clusterId,
+      @PathParam("instanceName") String instanceName){
+
+    try {
+      JsonNode node = null;
+      if (jsonContent.length() != 0) {
+        node = OBJECT_MAPPER.readTree(jsonContent);
+      }
+      if (node == null) {
+        return badRequest("Invalid input for content : " + jsonContent);
+      }
+      MaintenanceOpInputFields inputFields = readMaintenanceInputFromJson(node);
+      if (inputFields.healthChecks.size() != 0) {
+        LOG.warn("freeSingleInstance won't perform user passed health check.");
+      }
+
+      MaintenanceManagementService maintenanceManagementService =
+          new MaintenanceManagementService((ZKHelixDataAccessor) getDataAccssor(clusterId),
+              getConfigAccessor(), inputFields.skipZKRead, inputFields.continueOnFailures,
+              getNamespace());
+
+      return JSONRepresentation(maintenanceManagementService
+          .freeInstance(clusterId, instanceName, inputFields.healthChecks,
+              inputFields.healthCheckConfig,
+              inputFields.operations,
+              inputFields.operationConfig, true));
+    } catch (Exception e) {
+      LOG.error("Failed to takeInstances:", e);
+      return badRequest("Failed to takeInstances: " + e.getMessage());
+    }
+  }
+
+  private MaintenanceOpInputFields readMaintenanceInputFromJson(JsonNode node) throws IOException {
+    MaintenanceOpInputFields inputFields = new MaintenanceOpInputFields();
+    String continueOnFailuresName = PerInstanceProperties.continueOnFailures.name();
+    String skipZKReadName = PerInstanceProperties.skipZKRead.name();
+
+    inputFields.continueOnFailures =
+        inputFields.healthCheckConfig != null && inputFields.healthCheckConfig
+            .containsKey(continueOnFailuresName) && Boolean
+            .parseBoolean(inputFields.healthCheckConfig.get(continueOnFailuresName));
+    inputFields.skipZKRead = inputFields.healthCheckConfig != null && inputFields.healthCheckConfig
+        .containsKey(skipZKReadName) && Boolean
+        .parseBoolean(inputFields.healthCheckConfig.get(skipZKReadName));
+
+    inputFields.healthChecks = MaintenanceManagementService
+        .getListFromJsonPayload(node.get(PerInstanceProperties.health_check_list.name()));
+    inputFields.healthCheckConfig = MaintenanceManagementService
+        .getMapFromJsonPayload(node.get(PerInstanceProperties.health_check_config.name()));
+    if (inputFields.healthCheckConfig != null || !inputFields.healthChecks.isEmpty()) {
+      inputFields.healthCheckConfig.remove(continueOnFailuresName);
+      inputFields.healthCheckConfig.remove(skipZKReadName);
+    }
+
+    inputFields.operations = MaintenanceManagementService
+        .getListFromJsonPayload(node.get(PerInstanceProperties.operation_list.name()));
+    inputFields.operationConfig = MaintenanceManagementService
+        .getMapFromJsonPayload(node.get(PerInstanceProperties.operation_config.name()));
+
+    LOG.debug("healthChecks: " + (inputFields.healthChecks == null ? "NULL"

Review comment:
       Can we just use inputFields.toString()?




-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org