xiangfu0 commented on code in PR #17375:
URL: https://github.com/apache/pinot/pull/17375#discussion_r2664318321


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3602,6 +3634,48 @@ public PinotResourceManagerResponse dropInstance(String 
instanceName) {
     return PinotResourceManagerResponse.success("Instance " + instanceName + " 
dropped");
   }
 
+  /**
+   * Drains a minion instance by preventing new task assignments while 
allowing existing tasks to complete.
+   * This is achieved by replacing all instance tags with minion_drained. 
Since Helix uses containsTag()
+   * for task assignment matching, keeping any existing tags would still allow 
task assignments.
+   *
+   * @param instanceName Name of the minion instance to drain
+   * @return Response indicating success or failure
+   * @throws UnsupportedOperationException if the minion is already drained
+   */
+  public synchronized PinotResourceManagerResponse drainMinionInstance(String 
instanceName) {
+    InstanceConfig instanceConfig = getHelixInstanceConfig(instanceName);
+    if (instanceConfig == null) {
+      return PinotResourceManagerResponse.failure("Instance " + instanceName + 
" not found");
+    }
+
+    // Validate that minion is not already drained
+    List<String> currentTags = instanceConfig.getTags();
+    if (currentTags != null && 
currentTags.contains(Helix.DRAINED_MINION_INSTANCE)) {
+      throw new UnsupportedOperationException(

Review Comment:
   this exception won't be caught by the external caller in 
`pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java#toggleInstanceState()`
   
   I feel return is better:
   ```
   return PinotResourceManagerResponse.failure("Minion instance " + 
instanceName + " is already drained");
   ```



-- 
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]

Reply via email to