xyuanlu commented on code in PR #2149:
URL: https://github.com/apache/helix/pull/2149#discussion_r900529144


##########
helix-core/src/main/java/org/apache/helix/cloud/event/helix/DefaultCloudEventCallbackImpl.java:
##########
@@ -19,51 +19,102 @@
  * under the License.
  */
 
+import org.apache.helix.HelixDataAccessor;
 import org.apache.helix.HelixManager;
-import sun.reflect.generics.reflectiveObjects.NotImplementedException;
+import org.apache.helix.constants.InstanceConstants;
+import org.apache.helix.model.ClusterConfig;
+import org.apache.helix.util.InstanceValidationUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 
 /**
  * A default callback implementation class to be used in {@link 
HelixCloudEventListener}
  */
 public class DefaultCloudEventCallbackImpl {
+  private static final Logger LOG = 
LoggerFactory.getLogger(DefaultCloudEventCallbackImpl.class);
+  private final String _instanceReason = "Cloud event in 
DefaultCloudEventCallback at %s";
+  private final String _emmReason = "Cloud event EMM in 
DefaultCloudEventCallback by %s at %s";
 
   /**
-   * Disable the instance
+   * Disable the instance and track the cloud event in map field 
disabledInstancesWithInfo in
+   * cluster config. Will not re-disable the instance if the instance is 
already disabled for
+   * other reason. (So we will not overwrite the disabled reason and enable 
this instance when
+   * on-unpause)
    * @param manager The helix manager associated with the listener
    * @param eventInfo Detailed information about the event
    */
   public void disableInstance(HelixManager manager, Object eventInfo) {
-    // To be implemented
-    throw new NotImplementedException();
+    String message = String.format(_instanceReason, 
System.currentTimeMillis());
+    LOG.info("DefaultCloudEventCallbackImpl disabled Instance {}", 
manager.getInstanceName());
+    if (InstanceValidationUtil
+        .isEnabled(manager.getHelixDataAccessor(), manager.getInstanceName())) 
{
+      manager.getClusterManagmentTool()
+          .enableInstance(manager.getClusterName(), manager.getInstanceName(), 
false,
+              InstanceConstants.InstanceDisabledType.CLOUD_EVENT, message);
+    }

Review Comment:
   TFTR. I feel like adding another log won't help debug there is already log 
in ZkHelixManager.enableInstance. 



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