[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13855025#comment-13855025 ] Hudson commented on HADOOP-10169: - FAILURE: Integrated in Hadoop-Hdfs-trunk #1619 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk/1619/]) HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. Contributed by Liang Xie. (jing9: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820) * /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt * /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Fix For: 2.4.0 > > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13855013#comment-13855013 ] Hudson commented on HADOOP-10169: - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1645 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1645/]) HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. Contributed by Liang Xie. (jing9: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820) * /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt * /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Fix For: 2.4.0 > > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13854991#comment-13854991 ] Hudson commented on HADOOP-10169: - FAILURE: Integrated in Hadoop-Yarn-trunk #428 (See [https://builds.apache.org/job/Hadoop-Yarn-trunk/428/]) HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. Contributed by Liang Xie. (jing9: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820) * /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt * /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Fix For: 2.4.0 > > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13854982#comment-13854982 ] Hudson commented on HADOOP-10169: - SUCCESS: Integrated in Hadoop-trunk-Commit #4924 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/4924/]) HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. Contributed by Liang Xie. (jing9: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820) * /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt * /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Fix For: 2.4.0 > > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13854599#comment-13854599 ] Jing Zhao commented on HADOOP-10169: +1 for the latest patch. Thanks for cleaning the code, Liang Xie! And thanks to Jason for the review! I will commit it shortly. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13853623#comment-13853623 ] Hadoop QA commented on HADOOP-10169: {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12619729/HADOOP-10169-v4.txt against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:red}-1 tests included{color}. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 core tests{color}. The patch passed unit tests in hadoop-common-project/hadoop-common. {color:green}+1 contrib tests{color}. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3375//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3375//console This message is automatically generated. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13853590#comment-13853590 ] Liang Xie commented on HADOOP-10169: Yes, you are right, now attached v4 patch. Thanks for review, i really appreciate and enjoy it:) and have learned a lot from those detail code/styles. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169-v4.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13853023#comment-13853023 ] Jason Lowe commented on HADOOP-10169: - Thanks for updating the patch. One last nit is that this code: {code} if (gcInfo == null) { gcInfo = new MetricsInfo[2]; MetricsInfo[] previousGcInfo = gcInfoCache.putIfAbsent(gcName, gcInfo); if (previousGcInfo == null) { return gcInfo; } return previousGcInfo; } return gcInfo; {code} can be simplified to the following so there aren't so many return statements to track: {code} if (gcInfo == null) { gcInfo = new MetricsInfo[2]; MetricsInfo[] previousGcInfo = gcInfoCache.putIfAbsent(gcName, gcInfo); if (previousGcInfo != null) { gcInfo = previousGcInfo; } } return gcInfo; {code} > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852582#comment-13852582 ] Hadoop QA commented on HADOOP-10169: {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12619459/HADOOP-10169-v3.txt against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:red}-1 tests included{color}. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 core tests{color}. The patch passed unit tests in hadoop-common-project/hadoop-common. {color:green}+1 contrib tests{color}. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3370//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3370//console This message is automatically generated. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852501#comment-13852501 ] Liang Xie commented on HADOOP-10169: Thanks [~jlowe] for your nice reply, yes, you are correct! Attached v3 with an IDE formatting now:) > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, > HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13851875#comment-13851875 ] Jason Lowe commented on HADOOP-10169: - Doh, in my haste I misread the code, so my pseudo-code isn't relevant. We're not initializing the map just once overall, we're initializing it once per key. As you point out, we can be initializing a new key as we're busy accessing an old key, and something like a ConcurrentMap is more appropriate. However in general we cannot just replace HashMap with ConcurrentHashMap, remove the synchronized keywords, and expect it to work properly in all cases. There's now a race in getGcInfo where thread A comes along, sees there isn't an entry in the map for key K, and starts creating an empty MetricsInfo for it. Meanwhile thread B comes along, also sees there isn't an entry for key K, creates an empty MetricsInfo, puts it in the map, updates the MetricsInfo with new metrics, and continues on. Thread A then wakes back up and pokes the empty MetricsInfo into the map for key K, causing data loss of the metrics computed by thread B. The gcInfoCache.put needs to be gcInfoCache.putIfAbsent, and if putIfAbsent returns a value then we need to return that instead of the empty metrics info. Couple of other nits on the patch: the com.google.common.collect.Maps import is no longer necessary, and the patch includes an unrelated whitespace change that pushes one of the modified lines over 80 columns. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13851359#comment-13851359 ] Hadoop QA commented on HADOOP-10169: {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12619238/HADOOP-10169-v2.txt against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:red}-1 tests included{color}. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 core tests{color}. The patch passed unit tests in hadoop-common-project/hadoop-common. {color:green}+1 contrib tests{color}. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3366//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3366//console This message is automatically generated. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13851303#comment-13851303 ] Liang Xie commented on HADOOP-10169: [~jingzhao], [~jlowe], thanks for your review, yes, seems my original thought has some flaw:) I just attached another small v2 patch, could you help to review again? thanks in advance! I think it's enough to use the ConcurrentHashMap to replace the big Synchronized, and it should be OK for all the read-only requests once initialization done. [~jlowe], thanks for your detailed pseudo-code, IMHO, it seems a bit tricky here to work correctly maybe, since the getGcInfo() is invoked by a loop in getGcUsage(), then if the first time in loop initialized successful, then the latter ones will be omitted:) > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850514#comment-13850514 ] Jason Lowe commented on HADOOP-10169: - As Jing pointed out, the function needs to be synchronized as written. It's true that once initialized the code will no longer modify the map, but there's a race during the initialization itself. If two or more threads call this function before it's initialized then they can both attempt to put() at the same time. Alternatively, a thread could be late to the party and be calling get() just as the initializing thread is calling put() which is also a thread safety violation. One possible way to avoid the synchronization bottleneck is to use a volatile boolean to indicate whether it's initialized so the vast majority of callers don't have to grab a lock. Something like this pseudo-code: {code} volatile boolean isInitialized = false; if (!isInitialized) { synchronized (gcInfoCache) { if (!isInitialized) { . gcInfoCache.put(...); } } isInitialized = true; } return gcInfoCache.get(...); {code} > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850242#comment-13850242 ] Liang Xie commented on HADOOP-10169: The map gcInfoCache will be filled up once the first getMetrics() be invoked, seems then it'll be keep unchanged always. Then during almost of all the life cycle, the gcInfoCache will be read only without any write. Do we still need the synchronized? Am i missing sth ? thanks for taking a look at it:) > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850209#comment-13850209 ] Jing Zhao commented on HADOOP-10169: Is it possible that multiple threads call getMetrics(MetricsCollector, boolean) around the same time, which causes getGcInfo to be called concurrently? In that case, we still need the synchronized here? > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)
[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class
[ https://issues.apache.org/jira/browse/HADOOP-10169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13850158#comment-13850158 ] Hadoop QA commented on HADOOP-10169: {color:red}-1 overall{color}. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12619040/HADOOP-10169.txt against trunk revision . {color:green}+1 @author{color}. The patch does not contain any @author tags. {color:red}-1 tests included{color}. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color:green}+1 javac{color}. The applied patch does not increase the total number of javac compiler warnings. {color:green}+1 javadoc{color}. The javadoc tool did not generate any warning messages. {color:green}+1 eclipse:eclipse{color}. The patch built with eclipse:eclipse. {color:green}+1 findbugs{color}. The patch does not introduce any new Findbugs (version 1.3.9) warnings. {color:green}+1 release audit{color}. The applied patch does not increase the total number of release audit warnings. {color:green}+1 core tests{color}. The patch passed unit tests in hadoop-common-project/hadoop-common. {color:green}+1 contrib tests{color}. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3363//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3363//console This message is automatically generated. > remove the unnecessary synchronized in JvmMetrics class > > > Key: HADOOP-10169 > URL: https://issues.apache.org/jira/browse/HADOOP-10169 > Project: Hadoop Common > Issue Type: Improvement > Components: metrics >Affects Versions: 3.0.0, 2.2.0 >Reporter: Liang Xie >Assignee: Liang Xie >Priority: Minor > Attachments: HADOOP-10169.txt > > > When i looked into a HBase JvmMetric impl, just found this synchronized seems > not essential. -- This message was sent by Atlassian JIRA (v6.1.4#6159)