[
https://issues.apache.org/jira/browse/HBASE-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14718316#comment-14718316
]
Hiroshi Ikeda commented on HBASE-14279:
---------------------------------------
There is a similar class, {{KeyLoccker}}, whose key type is a generic one, and
you can use it without copying its implementation logic. The implementation of
{{IdLocker}} and {{KeyLocker}} is not good and should be re-written, and I
believe it is better to avoid to copy their implementation.
To begin with, using 2 objects of {{ConcurrentHashMap}} is not good because
{{putIfAbset}} and {{remove}} use just lock striping internally, and the
periods of time which may cause conflict between threads increases twofold. It
is better to manually use lock striping instead of {{ConcurrentHashMap}}.
BTW, {{ConcurrentHashMap.get}} just locks the segment only when accessing a
value, and it is an idem to use {{get}} before {{putIfAbsent}}. This has
potentially to reduce granularity of locks, though it seems not trivial to
implement.
There is another thing to concern about. From the point of the view of
object-oriented programing, exposing internal values by the method {{values}}
causes breaking the invariant that the empty set should be removed from the
map. It should be defensively copied, though it is required to change the API,
and it may also be required to change code of the clients which use this class,
making the patch easier to become stale.
> Race condition in ConcurrentIndex
> ---------------------------------
>
> Key: HBASE-14279
> URL: https://issues.apache.org/jira/browse/HBASE-14279
> Project: HBase
> Issue Type: Bug
> Reporter: Hiroshi Ikeda
> Assignee: Heng Chen
> Priority: Minor
> Attachments: HBASE-14279.patch, HBASE-14279_v2.patch
>
>
> {{ConcurrentIndex.put}} and {{remove}} are in race condition. It is possible
> to remove a non-empty set, and to add a value to a removed set. Also
> {{ConcurrentIndex.values}} is vague in sense that the returned set sometimes
> trace the current state and sometimes doesn't.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
