[GitHub] [kafka] showuon commented on a diff in pull request #14287: [Minor] Check the existence of AppInfo for the given ID before creating a new mbean of the same name

[via GitHub]

showuon commented on code in PR #14287:
URL: https://github.com/apache/kafka/pull/14287#discussion_r1319285955


##########
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java:
##########
@@ -60,8 +60,13 @@ public static String getCommitId() {
     public static synchronized void registerAppInfo(String prefix, String id, 
Metrics metrics, long nowMs) {
         try {
             ObjectName name = new ObjectName(prefix + ":type=app-info,id=" + 
Sanitizer.jmxSanitize(id));
+            MBeanServer server = ManagementFactory.getPlatformMBeanServer();
+            if (server.isRegistered(name)) {
+                log.info("App info {} for {} already exists, so skipping  a 
new mbean creation", prefix, id);

Review Comment:
   nit: additional space after _skipping_.
   
   Question: What is this log output under consumer's case? I think it's:
   
   `App info kafka.consumer for transcript2_REALTIME-transcript-topic-0 already 
exists, so skipping a new mbean creation`
   
   Could we change to:
   `log.info("The mbean of App info: [{}], id: [{}] already exists, so skipping 
a new mbean creation.", prefix, id);`
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org