You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by gt...@apache.org on 2018/11/15 17:45:29 UTC

activemq git commit: AMQ-7102 - don't track objectNames that have not been registered due to suppressMBean filter, fix and test

Repository: activemq
Updated Branches:
  refs/heads/master 50d1fe9f8 -> 9cb680c0b


AMQ-7102 - don't track objectNames that have not been registered due to suppressMBean filter, fix and test


Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/9cb680c0
Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/9cb680c0
Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/9cb680c0

Branch: refs/heads/master
Commit: 9cb680c0bad9fb3f92807d0f49e02505c544e3e9
Parents: 50d1fe9
Author: gtully <ga...@gmail.com>
Authored: Thu Nov 15 17:45:18 2018 +0000
Committer: gtully <ga...@gmail.com>
Committed: Thu Nov 15 17:45:18 2018 +0000

----------------------------------------------------------------------
 .../broker/jmx/AsyncAnnotatedMBean.java         | 10 +++---
 .../broker/jmx/ManagedRegionBroker.java         | 34 +++++++++++++-------
 .../jmx/SelectiveMBeanRegistrationTest.java     | 22 +++++++++++++
 3 files changed, 49 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq/blob/9cb680c0/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/AsyncAnnotatedMBean.java
----------------------------------------------------------------------
diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/AsyncAnnotatedMBean.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/AsyncAnnotatedMBean.java
index 7871a21..bc7a826 100644
--- a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/AsyncAnnotatedMBean.java
+++ b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/AsyncAnnotatedMBean.java
@@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit;
 
 import javax.management.MBeanException;
 import javax.management.NotCompliantMBeanException;
+import javax.management.ObjectInstance;
 import javax.management.ObjectName;
 import javax.management.ReflectionException;
 
@@ -52,7 +53,7 @@ public class AsyncAnnotatedMBean extends AnnotatedMBean {
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static void registerMBean(ExecutorService executor, long timeout, ManagementContext context, Object object, ObjectName objectName) throws Exception {
+    public static ObjectInstance registerMBean(ExecutorService executor, long timeout, ManagementContext context, Object object, ObjectName objectName) throws Exception {
 
         if (timeout < 0 && executor != null) {
             throw new IllegalArgumentException("async timeout cannot be negative.");
@@ -67,15 +68,14 @@ public class AsyncAnnotatedMBean extends AnnotatedMBean {
         for (Class c : object.getClass().getInterfaces()) {
             if (mbeanName.equals(c.getName())) {
                 if (timeout == 0) {
-                    context.registerMBean(new AnnotatedMBean(object, c, objectName), objectName);
+                    return context.registerMBean(new AnnotatedMBean(object, c, objectName), objectName);
                 } else {
-                    context.registerMBean(new AsyncAnnotatedMBean(executor, timeout, object, c, objectName), objectName);
+                    return context.registerMBean(new AsyncAnnotatedMBean(executor, timeout, object, c, objectName), objectName);
                 }
-                return;
             }
         }
 
-        context.registerMBean(object, objectName);
+        return context.registerMBean(object, objectName);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/activemq/blob/9cb680c0/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedRegionBroker.java
----------------------------------------------------------------------
diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedRegionBroker.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedRegionBroker.java
index fa9584d..3eaf28b 100644
--- a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedRegionBroker.java
+++ b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedRegionBroker.java
@@ -324,8 +324,9 @@ public class ManagedRegionBroker extends RegionBroker {
             }
         }
         try {
-            AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key);
-            registeredMBeans.add(key);
+            if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key) != null) {
+                registeredMBeans.add(key);
+            }
         } catch (Throwable e) {
             LOG.warn("Failed to register MBean {}", key);
             LOG.debug("Failure reason: ", e);
@@ -380,8 +381,9 @@ public class ManagedRegionBroker extends RegionBroker {
         }
 
         try {
-            AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key);
-            registeredMBeans.add(key);
+            if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key) != null) {
+                registeredMBeans.add(key);
+            }
         } catch (Throwable e) {
             LOG.warn("Failed to register MBean {}", key);
             LOG.debug("Failure reason: ", e);
@@ -444,8 +446,9 @@ public class ManagedRegionBroker extends RegionBroker {
         }
 
         try {
-            AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key);
-            registeredMBeans.add(key);
+            if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, key) != null) {
+                registeredMBeans.add(key);
+            }
         } catch (Throwable e) {
             LOG.warn("Failed to register MBean {}", key);
             LOG.debug("Failure reason: ", e);
@@ -520,8 +523,9 @@ public class ManagedRegionBroker extends RegionBroker {
             SubscriptionView view = new InactiveDurableSubscriptionView(this, brokerService, key.getClientId(), info, subscription);
 
             try {
-                AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName);
-                registeredMBeans.add(objectName);
+                if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName) != null) {
+                    registeredMBeans.add(objectName);
+                }
             } catch (Throwable e) {
                 LOG.warn("Failed to register MBean {}", key);
                 LOG.debug("Failure reason: ", e);
@@ -770,8 +774,9 @@ public class ManagedRegionBroker extends RegionBroker {
                     view = new AbortSlowConsumerStrategyView(this, strategy);
                 }
 
-                AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName);
-                registeredMBeans.add(objectName);
+                if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName) != null) {
+                    registeredMBeans.add(objectName);
+                }
             }
         } catch (Exception e) {
             LOG.warn("Failed to register MBean {}", strategy);
@@ -785,8 +790,9 @@ public class ManagedRegionBroker extends RegionBroker {
             ObjectName objectName = BrokerMBeanSupport.createXATransactionName(brokerObjectName, transaction);
             if (!registeredMBeans.contains(objectName))  {
                 RecoveredXATransactionView view = new RecoveredXATransactionView(this, transaction);
-                AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName);
-                registeredMBeans.add(objectName);
+                if (AsyncAnnotatedMBean.registerMBean(asyncInvokeService, mbeanTimeout, managementContext, view, objectName) != null) {
+                    registeredMBeans.add(objectName);
+                }
             }
         } catch (Exception e) {
             LOG.warn("Failed to register prepared transaction MBean {}", transaction);
@@ -837,4 +843,8 @@ public class ManagedRegionBroker extends RegionBroker {
         ObjectName objName = BrokerMBeanSupport.createDestinationName(brokerObjectName.toString(), "Queue", queueName);
         return queues.get(objName);
     }
+
+    public Set<ObjectName> getRegisteredMbeans() {
+        return registeredMBeans;
+    }
 }

http://git-wip-us.apache.org/repos/asf/activemq/blob/9cb680c0/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/SelectiveMBeanRegistrationTest.java
----------------------------------------------------------------------
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/SelectiveMBeanRegistrationTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/SelectiveMBeanRegistrationTest.java
index a1c6aec..b6ea4a0 100644
--- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/SelectiveMBeanRegistrationTest.java
+++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/SelectiveMBeanRegistrationTest.java
@@ -74,6 +74,11 @@ public class SelectiveMBeanRegistrationTest  {
 
         session.createConsumer(queue);
 
+        // create a plain topic
+        Destination topic = session.createTopic("ATopic");
+        session.createConsumer(topic);
+
+
         final ManagedRegionBroker managedRegionBroker = (ManagedRegionBroker) brokerService.getBroker().getAdaptor(ManagedRegionBroker.class);
 
         // mbean exists
@@ -87,6 +92,10 @@ public class SelectiveMBeanRegistrationTest  {
         // but it is not registered
         assertFalse(mbeanServer.isRegistered(managedRegionBroker.getQueueSubscribers()[0]));
 
+        // and is not tracked
+        assertFalse("not tracked as registered", managedRegionBroker.getRegisteredMbeans().contains(managedRegionBroker.getQueueSubscribers()[0]));
+
+
         // verify dynamicProducer suppressed
         session.createProducer(null);
 
@@ -105,9 +114,22 @@ public class SelectiveMBeanRegistrationTest  {
         Set<ObjectInstance> mbeans = mbeanServer.queryMBeans(query, null);
         assertEquals(0, mbeans.size());
 
+        assertFalse("producer not tracked as registered", managedRegionBroker.getRegisteredMbeans().contains(managedRegionBroker.getDynamicDestinationProducers()[0]));
+
+
         query = new ObjectName(domain + ":type=Broker,brokerName=localhost,destinationName=ActiveMQ.Advisory.*,*");
         mbeans = mbeanServer.queryMBeans(query, null);
         assertEquals(0, mbeans.size());
+
+        ObjectName[] topicNames = managedRegionBroker.getTopics();
+        assertTrue("Some topics registered", topicNames.length > 0);
+        for (ObjectName objectName : topicNames) {
+            if (objectName.getKeyProperty("destinationName").contains("Advisory")) {
+                assertFalse("advisory topic not tracked as registered: " + objectName, managedRegionBroker.getRegisteredMbeans().contains(objectName));
+            } else {
+                assertTrue("topic tracked as registered: " + objectName, managedRegionBroker.getRegisteredMbeans().contains(objectName));
+            }
+        }
     }