KAFKA-4856: make unregisterAppInfo method to synchronized #3696
Conversation
|
minor fix to avoid error logs |
| @@ -61,7 +61,7 @@ public static void registerAppInfo(String prefix, String id) { | |||
| } | |||
| } | |||
|
|
|||
| public static void unregisterAppInfo(String prefix, String id) { | |||
| public static synchronized void unregisterAppInfo(String prefix, String id) { | |||
There was a problem hiding this comment.
If this must be synchronized, don't we have to do the same for registerAppInfo?
There was a problem hiding this comment.
yes for consistency, we can synchronize registerAppInfo method also. when multiple clients tries register mbean with the same clientId, we print an error message and continue. when multiple threads try to unregister we delete only if mbean exist in the mbean registry.
updated the PR.
There was a problem hiding this comment.
Btw, we should take the chance and make version and commitId final. Something like:
private static final String version;
private static final String commitId;
static {
Properties props = new Properties();
try (InputStream resourceStream = AppInfoParser.class.getResourceAsStream("/kafka/kafka-version.properties")) {
props.load(resourceStream);
} catch (Exception e) {
log.warn("Error while loading kafka-version.properties :" + e.getMessage());
}
version = props.getProperty("version", "unknown").trim();
commitId = props.getProperty("commitId", "unknown").trim();
}There was a problem hiding this comment.
Thanks. Updated the changes.
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
retest this please |
And make AppInfo's static fields final. Author: Manikumar Reddy <manikumar.reddy@gmail.com> Reviewers: Ismael Juma <ismael@juma.me.uk> Closes #3696 from omkreddy/KAFKA-4856 (cherry picked from commit 6cf92a2) Signed-off-by: Ismael Juma <ismael@juma.me.uk>
No description provided.