[pinot] branch master updated: Instance drop operation validation check API (#10969)

[sn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

snlee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 878f61357b Instance drop operation validation check API (#10969)
878f61357b is described below

commit 878f61357b6868d98a3eb8c84ec1dfb414c44b6f
Author: Shounak kulkarni <sh...@gmail.com>
AuthorDate: Thu Jul 6 11:51:21 2023 +0530

    Instance drop operation validation check API (#10969)
    
    * extract instance drop validity check code to a separate method
    
    * Add javadoc
    
    * API endpoint to expose the safety validation on instance drop operation
    
    * lint fix
    
    * add license header to new class
    
    * update the endpoint path and method
    
    * pass error list as code, message tuple
---
 .../api/resources/OperationValidationResponse.java | 93 ++++++++++++++++++++++
 .../resources/PinotInstanceRestletResource.java    | 26 ++++++
 .../helix/core/PinotHelixResourceManager.java      | 43 ++++++----
 3 files changed, 148 insertions(+), 14 deletions(-)

diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/OperationValidationResponse.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/OperationValidationResponse.java
new file mode 100644
index 0000000000..43897ccdc0
--- /dev/null
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/OperationValidationResponse.java
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.api.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.ArrayList;
+import java.util.List;
+
+
+public class OperationValidationResponse {
+  private String _instanceName;
+  private boolean _safe;
+  private final List<ErrorWrapper> _issues = new ArrayList<>();
+
+  @JsonProperty("instanceName")
+  public String getInstanceName() {
+    return _instanceName;
+  }
+
+  public OperationValidationResponse setInstanceName(String instanceName) {
+    _instanceName = instanceName;
+    return this;
+  }
+
+  @JsonProperty("isSafe")
+  public boolean isSafe() {
+    return _safe;
+  }
+
+  public OperationValidationResponse setSafe(boolean safe) {
+    _safe = safe;
+    return this;
+  }
+
+  @JsonProperty("issues")
+  public List<ErrorWrapper> getIssues() {
+    return _issues;
+  }
+
+  public OperationValidationResponse putIssue(ErrorCode code, String... args) {
+    _issues.add(new ErrorWrapper(code, args));
+    return this;
+  }
+
+  public String getIssueMessage(int index) {
+    return _issues.get(index).getMessage();
+  }
+
+  public static class ErrorWrapper {
+    ErrorCode _code;
+    String _message;
+
+    public ErrorWrapper(ErrorCode code, String... args) {
+      _code = code;
+      _message = String.format(code._description, args);
+    }
+
+    public ErrorCode getCode() {
+      return _code;
+    }
+
+    public String getMessage() {
+      return _message;
+    }
+  }
+
+  public enum ErrorCode {
+    IS_ALIVE("Instance %s is still live"),
+    CONTAINS_RESOURCE("Instance %s exists in ideal state for %s");
+
+    public final String _description;
+
+    ErrorCode(String description) {
+      _description = description;
+    }
+  }
+}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
index cff4a470a4..5752599d0a 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
@@ -31,6 +31,7 @@ import io.swagger.annotations.SecurityDefinition;
 import io.swagger.annotations.SwaggerDefinition;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.ws.rs.ClientErrorException;
 import javax.ws.rs.Consumes;
@@ -390,4 +391,29 @@ public class PinotInstanceRestletResource {
           Response.Status.INTERNAL_SERVER_ERROR, e);
     }
   }
+
+  @GET
+  @Path("/instances/dropInstance/validate")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Check if it's safe to drop the given instances. If not list all the reasons why its not safe.")
+  @ApiResponses(value = {
+      @ApiResponse(code = 200, message = "Success"),
+      @ApiResponse(code = 500, message = "Internal error")
+  })
+  public List<OperationValidationResponse> instanceDropSafetyCheck(
+      @ApiParam(value = "Instance names", required = true,
+          example = "Broker_my.broker.com_30000")
+      @QueryParam("instanceNames") List<String> instanceNames) {
+    LOGGER.info("Performing safety check on drop operation request received for instances: {}", instanceNames);
+    try {
+      return instanceNames.stream()
+              .map(instance -> _pinotHelixResourceManager.instanceDropSafetyCheck(instance))
+              .collect(Collectors.toList());
+    } catch (ClientErrorException e) {
+      throw new ControllerApplicationException(LOGGER, e.getMessage(), e.getResponse().getStatus());
+    } catch (Exception e) {
+      throw new ControllerApplicationException(LOGGER, "Failed to check the safety for instance drop operation.",
+          Response.Status.INTERNAL_SERVER_ERROR, e);
+    }
+  }
 }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
index e18eeff927..fdbca20433 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
@@ -134,6 +134,7 @@ import org.apache.pinot.controller.api.exception.InvalidTableConfigException;
 import org.apache.pinot.controller.api.exception.TableAlreadyExistsException;
 import org.apache.pinot.controller.api.exception.UserAlreadyExistsException;
 import org.apache.pinot.controller.api.resources.InstanceInfo;
+import org.apache.pinot.controller.api.resources.OperationValidationResponse;
 import org.apache.pinot.controller.api.resources.PeriodicTaskInvocationResponse;
 import org.apache.pinot.controller.api.resources.StateType;
 import org.apache.pinot.controller.helix.core.assignment.instance.InstanceAssignmentDriver;
@@ -3122,20 +3123,9 @@ public class PinotHelixResourceManager {
    * </ul>
    */
   public PinotResourceManagerResponse dropInstance(String instanceName) {
-    // Check if the instance is live
-    if (_helixDataAccessor.getProperty(_keyBuilder.liveInstance(instanceName)) != null) {
-      return PinotResourceManagerResponse.failure("Instance " + instanceName + " is still live");
-    }
-
-    // Check if any ideal state includes the instance
-    for (String resource : getAllResources()) {
-      IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, resource);
-      for (String partition : idealState.getPartitionSet()) {
-        if (idealState.getInstanceSet(partition).contains(instanceName)) {
-          return PinotResourceManagerResponse.failure(
-              "Instance " + instanceName + " exists in ideal state for " + resource);
-        }
-      }
+    OperationValidationResponse check = instanceDropSafetyCheck(instanceName);
+    if (!check.isSafe()) {
+      return PinotResourceManagerResponse.failure(check.getIssueMessage(0));
     }
 
     // Remove '/INSTANCES/<instanceName>'
@@ -3157,6 +3147,31 @@ public class PinotHelixResourceManager {
     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 {@link OperationValidationResponse}
+   */
+  public OperationValidationResponse instanceDropSafetyCheck(String instanceName) {
+    OperationValidationResponse response = new OperationValidationResponse().setInstanceName(instanceName);
+    // Check if the instance is live
+    if (_helixDataAccessor.getProperty(_keyBuilder.liveInstance(instanceName)) != null) {
+      response.putIssue(OperationValidationResponse.ErrorCode.IS_ALIVE, instanceName);
+    }
+    // Check if any ideal state includes the instance
+    getAllResources().forEach(resource -> {
+      IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, resource);
+      for (String partition : idealState.getPartitionSet()) {
+        if (idealState.getInstanceSet(partition).contains(instanceName)) {
+          response.putIssue(OperationValidationResponse.ErrorCode.CONTAINS_RESOURCE, instanceName, resource);
+          break;
+        }
+      }
+    });
+    return response.setSafe(response.getIssues().isEmpty());
+  }
+
   /**
    * Toggle the status of an Instance between OFFLINE and ONLINE.
    * Keeps checking until ideal-state is successfully updated or times out.


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