[kafka] branch trunk updated: KAFKA-6307: Fix KafkaMbean leak in JmxReporter (#4307)

[ij...@apache.org]

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

ijuma pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 5d4965b  KAFKA-6307: Fix KafkaMbean leak in JmxReporter (#4307)
5d4965b is described below

commit 5d4965bad4ea4a64d798f1cc0271de8d4e3d4289
Author: tedyu <yu...@gmail.com>
AuthorDate: Mon Jan 1 02:16:07 2018 -0800

    KAFKA-6307: Fix KafkaMbean leak in JmxReporter (#4307)
    
    We should remove the map entry from mbeans if it becomes
    empty during metric removal.
    
    Reviewers: Manikumar Reddy <ma...@gmail.com>, Satish Duggana <sa...@gmail.com>, Ismael Juma <is...@juma.me.uk>
---
 .../java/org/apache/kafka/common/metrics/JmxReporter.java | 15 ++++++++++-----
 .../org/apache/kafka/common/metrics/JmxReporterTest.java  | 14 +++++++++++---
 2 files changed, 21 insertions(+), 8 deletions(-)

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 0c49224..063fb3b 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 class JmxReporter implements MetricsReporter {
         }
     }
 
+    boolean containsMbean(String mbeanName) {
+        return mbeans.containsKey(mbeanName);
+    }
     @Override
     public void metricChange(KafkaMetric metric) {
         synchronized (LOCK) {
@@ -86,19 +89,21 @@ public class JmxReporter implements MetricsReporter {
     @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 98e49f3..28179f3 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 class JmxReporterTest {
         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 class JmxReporterTest {
             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")));

-- 
To stop receiving notification emails like this one, please contact
['"commits@kafka.apache.org" <co...@kafka.apache.org>'].