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

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

satishd commented on a change in pull request #4307: KAFKA-6307 mBeanName 
should be removed before returning from JmxReporter#removeAttribute()
URL: https://github.com/apache/kafka/pull/4307#discussion_r158506203
 
 

 ##########
 File path: 
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java
 ##########
 @@ -49,7 +49,7 @@
     private static final Logger log = 
LoggerFactory.getLogger(JmxReporter.class);
     private static final Object LOCK = new Object();
     private String prefix;
-    private final Map<String, KafkaMbean> mbeans = new HashMap<String, 
KafkaMbean>();
+    final Map<String, KafkaMbean> mbeans = new HashMap<String, KafkaMbean>();
 
 Review comment:
   We may not want to expose this map to outside of this class as it can be 
modified using put/remove. Oneway to address is adding a method returning 
unmodifiable map. But it may be good to restrict with package level access 
method like `containsMBean(String mbeanName)` as the required test needs to 
know whether a mbean with a name exists or not.

----------------------------------------------------------------
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:
us...@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)

Reply via email to