[helix] 03/03: Ignore register mbean request if the MBean has been registered.

[jx...@apache.org]

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

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

commit 05a1286bdb48ccf7f8d0c6d80f783c8f85379839
Author: Jiajun Wang <jj...@linkedin.com>
AuthorDate: Wed Feb 6 18:02:33 2019 -0800

    Ignore register mbean request if the MBean has been registered.
    
    This is mainly for reducing unnessary logs.
    The additional change is to support more flexible MBean provider usage. We used to restrict a provider to be single use. If multiple registers happen, a warning message will be printed. This logic is no longer necessary with the previous DynamicMBeanProvider simplification and improvements.
---
 .../helix/monitoring/mbeans/MBeanRegistrar.java    |  2 +
 .../mbeans/dynamicMBeans/DynamicMBeanProvider.java | 43 +++++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/MBeanRegistrar.java b/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/MBeanRegistrar.java
index 31b7388..00db47f 100644
--- a/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/MBeanRegistrar.java
+++ b/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/MBeanRegistrar.java
@@ -52,6 +52,7 @@ public class MBeanRegistrar {
       num++;
       try {
         _beanServer.registerMBean(object, newObjectName);
+        LOG.info("MBean {} has been registered.", newObjectName.getCanonicalName());
       } catch (InstanceAlreadyExistsException e) {
         continue;
       } catch (JMException e) {
@@ -81,6 +82,7 @@ public class MBeanRegistrar {
     if (objectName != null && _beanServer.isRegistered(objectName)) {
       try {
         _beanServer.unregisterMBean(objectName);
+        LOG.info("MBean {} has been un-registered.", objectName.getCanonicalName());
       } catch (JMException e) {
         LOG.warn("Error in un-registering: " + objectName.getCanonicalName(), e);
       }
diff --git a/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/dynamicMBeans/DynamicMBeanProvider.java b/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/dynamicMBeans/DynamicMBeanProvider.java
index fbbb9e6..4299159 100644
--- a/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/dynamicMBeans/DynamicMBeanProvider.java
+++ b/helix-core/src/main/java/org/apache/helix/monitoring/mbeans/dynamicMBeans/DynamicMBeanProvider.java
@@ -19,15 +19,32 @@ package org.apache.helix.monitoring.mbeans.dynamicMBeans;
  * under the License.
  */
 
-import org.apache.helix.HelixException;
+import javax.management.Attribute;
+import javax.management.AttributeList;
+import javax.management.AttributeNotFoundException;
+import javax.management.DynamicMBean;
+import javax.management.InvalidAttributeValueException;
+import javax.management.JMException;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanConstructorInfo;
+import javax.management.MBeanException;
+import javax.management.MBeanInfo;
+import javax.management.MBeanNotificationInfo;
+import javax.management.MBeanOperationInfo;
+import javax.management.ObjectName;
+import javax.management.ReflectionException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.helix.monitoring.SensorNameProvider;
 import org.apache.helix.monitoring.mbeans.MBeanRegistrar;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.management.*;
-import java.util.*;
-
 /**
  * Dynamic MBean provider that reporting DynamicMetric attributes
  */
@@ -40,7 +57,7 @@ public abstract class DynamicMBeanProvider implements DynamicMBean, SensorNamePr
 
   // Attribute name to the DynamicMetric object mapping
   private final Map<String, DynamicMetric> _attributeMap = new HashMap<>();
-  private ObjectName _objectName;
+  private ObjectName _objectName = null;
   private MBeanInfo _mBeanInfo;
 
   /**
@@ -53,13 +70,8 @@ public abstract class DynamicMBeanProvider implements DynamicMBean, SensorNamePr
    */
   protected synchronized boolean doRegister(Collection<DynamicMetric<?, ?>> dynamicMetrics,
       String description, String domain, String... keyValuePairs) throws JMException {
-    if (_objectName != null) {
-      _logger.warn("Mbean has been registered before. Please create new object for new registration.");
-      return false;
-    }
-    updateAttributtInfos(dynamicMetrics, description);
-    _objectName = MBeanRegistrar.register(this, domain, keyValuePairs);
-    return true;
+    return doRegister(dynamicMetrics, description,
+        MBeanRegistrar.buildObjectName(domain, keyValuePairs));
   }
 
   /**
@@ -72,7 +84,8 @@ public abstract class DynamicMBeanProvider implements DynamicMBean, SensorNamePr
   protected synchronized boolean doRegister(Collection<DynamicMetric<?, ?>> dynamicMetrics,
       String description, ObjectName objectName) throws JMException {
     if (_objectName != null) {
-      _logger.warn("Mbean has been registered before. Please create new object for new registration.");
+      _logger.debug("Mbean {} has already been registered. Ignore register request.",
+          objectName.getCanonicalName());
       return false;
     }
     updateAttributtInfos(dynamicMetrics, description);
@@ -136,10 +149,12 @@ public abstract class DynamicMBeanProvider implements DynamicMBean, SensorNamePr
   public abstract DynamicMBeanProvider register() throws JMException;
 
   /**
-   * After unregistered, the MBean can't be registered again, a new monitor has to be created.
+   * Unregister the MBean and clean up object name record.
+   * Note that all the metric data is kept even after unregister.
    */
   public synchronized void unregister() {
     MBeanRegistrar.unregister(_objectName);
+    _objectName = null;
   }
 
   @Override