You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "snleee (via GitHub)" <gi...@apache.org> on 2023/06/30 08:37:28 UTC

[GitHub] [pinot] snleee commented on a diff in pull request #10969: Instance drop operation validation check API

snleee commented on code in PR #10969:
URL: https://github.com/apache/pinot/pull/10969#discussion_r1247597496


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java:
##########
@@ -390,4 +391,33 @@ public SuccessResponse updateBrokerResource(
           Response.Status.INTERNAL_SERVER_ERROR, e);
     }
   }
+
+  @POST
+  @Path("/instances/safetyCheck/dropInstance")

Review Comment:
   `/instances/dropInstance/validate` maybe a better path?
   
   We do have `POST /schemas/validate` & `POST /tables/validate`



##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3148,6 +3137,31 @@ public PinotResourceManagerResponse dropInstance(String instanceName) {
     return PinotResourceManagerResponse.success("Instance " + instanceName + " dropped");
   }
 
+  /**
+   * Utility to perform a safety check of the operation to drop an instance.
+   * If the resource is not safe to drop the utility lists all the possible reasons.
+   * @param instanceName Pinot instance name
+   * @return List of reasons why the operation is unsafe. Empty list if the operation is safe.
+   */
+  public List<String> instanceDropSafetyCheck(String instanceName) {
+    List<String> issues = new ArrayList<>();
+    // Check if the instance is live
+    if (_helixDataAccessor.getProperty(_keyBuilder.liveInstance(instanceName)) != null) {
+      issues.add(String.format("Instance %s is still live", instanceName));

Review Comment:
   +1 on this change. We can probably use some error code or enum status. If we want to add the explanation, we can model the response as `code/status, message`



-- 
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-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org