kafka-jira mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KAFKA-6307) mBeanName should be removed before returning from JmxReporter#removeAttribute()
Date Mon, 01 Jan 2018 10:17:00 GMT

    [ https://issues.apache.org/jira/browse/KAFKA-6307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16307410#comment-16307410
] 

ASF GitHub Bot commented on KAFKA-6307:
---------------------------------------

ijuma closed pull request #4307: KAFKA-6307 mBeanName should be removed before returning from
JmxReporter#removeAttribute()
URL: https://github.com/apache/kafka/pull/4307
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java b/clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java
index 0c49224657c..063fb3b9338 100644
--- a/clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java
+++ b/clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java
@@ -75,6 +75,9 @@ public void init(List<KafkaMetric> metrics) {
         }
     }
 
+    boolean containsMbean(String mbeanName) {
+        return mbeans.containsKey(mbeanName);
+    }
     @Override
     public void metricChange(KafkaMetric metric) {
         synchronized (LOCK) {
@@ -86,19 +89,21 @@ public void metricChange(KafkaMetric metric) {
     @Override
     public void metricRemoval(KafkaMetric metric) {
         synchronized (LOCK) {
-            KafkaMbean mbean = removeAttribute(metric);
+            MetricName metricName = metric.metricName();
+            String mBeanName = getMBeanName(prefix, metricName);
+            KafkaMbean mbean = removeAttribute(metric, mBeanName);
             if (mbean != null) {
-                if (mbean.metrics.isEmpty())
+                if (mbean.metrics.isEmpty()) {
                     unregister(mbean);
-                else
+                    mbeans.remove(mBeanName);
+                } else
                     reregister(mbean);
             }
         }
     }
 
-    private KafkaMbean removeAttribute(KafkaMetric metric) {
+    private KafkaMbean removeAttribute(KafkaMetric metric, String mBeanName) {
         MetricName metricName = metric.metricName();
-        String mBeanName = getMBeanName(prefix, metricName);
         KafkaMbean mbean = this.mbeans.get(mBeanName);
         if (mbean != null)
             mbean.removeAttribute(metricName.name());
diff --git a/clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java b/clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java
index 98e49f3abf5..28179f319a4 100644
--- a/clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.kafka.common.metrics;
 
+import org.apache.kafka.common.MetricName;
 import org.apache.kafka.common.metrics.stats.Avg;
 import org.apache.kafka.common.metrics.stats.Total;
 import org.junit.Test;
@@ -35,7 +36,8 @@ public void testJmxRegistration() throws Exception {
         Metrics metrics = new Metrics();
         MBeanServer server = ManagementFactory.getPlatformMBeanServer();
         try {
-            metrics.addReporter(new JmxReporter());
+            JmxReporter reporter = new JmxReporter();
+            metrics.addReporter(reporter);
 
             assertFalse(server.isRegistered(new ObjectName(":type=grp1")));
 
@@ -48,13 +50,19 @@ public void testJmxRegistration() throws Exception {
             assertTrue(server.isRegistered(new ObjectName(":type=grp2")));
             assertEquals(0.0, server.getAttribute(new ObjectName(":type=grp2"), "pack.bean2.total"));
 
-            metrics.removeMetric(metrics.metricName("pack.bean1.avg", "grp1"));
+            MetricName metricName = metrics.metricName("pack.bean1.avg", "grp1");
+            String mBeanName = JmxReporter.getMBeanName("", metricName);
+            assertTrue(reporter.containsMbean(mBeanName));
+            metrics.removeMetric(metricName);
+            assertFalse(reporter.containsMbean(mBeanName));
 
             assertFalse(server.isRegistered(new ObjectName(":type=grp1")));
             assertTrue(server.isRegistered(new ObjectName(":type=grp2")));
             assertEquals(0.0, server.getAttribute(new ObjectName(":type=grp2"), "pack.bean2.total"));
 
-            metrics.removeMetric(metrics.metricName("pack.bean2.total", "grp2"));
+            metricName = metrics.metricName("pack.bean2.total", "grp2");
+            metrics.removeMetric(metricName);
+            assertFalse(reporter.containsMbean(mBeanName));
 
             assertFalse(server.isRegistered(new ObjectName(":type=grp1")));
             assertFalse(server.isRegistered(new ObjectName(":type=grp2")));


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> mBeanName should be removed before returning from JmxReporter#removeAttribute()
> -------------------------------------------------------------------------------
>
>                 Key: KAFKA-6307
>                 URL: https://issues.apache.org/jira/browse/KAFKA-6307
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: siva santhalingam
>
> JmxReporter$KafkaMbean showed up near the top in the first histo output from KAFKA-6199.
> In JmxReporter#removeAttribute() :
> {code}
>         KafkaMbean mbean = this.mbeans.get(mBeanName);
>         if (mbean != null)
>             mbean.removeAttribute(metricName.name());
>         return mbean;
> {code}
> mbeans.remove(mBeanName) should be called before returning.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message