[
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)
