[
https://issues.apache.org/jira/browse/HBASE-11707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14092767#comment-14092767
]
Liu Shaohui commented on HBASE-11707:
-------------------------------------
[~liochon]
{quote}
You had this issue in production?
{quote}
No, just revisit the code and find this problem.
{quote}
The access to the list are protected by a synchronized at the method level, so
we don't need the volatile or the concurrentMap (or you could remove the
synchronized and check that there is no nasty race conditions).
{quote}
The isFailedServer and addToFailedServers method may run concurently, so i use
the concurrentMap.
{quote}
It seems we're going to clear the map all the time if there are no failures. Is
it cheap to clear an empty map?
{quote}
No. If there are no failures the map will not be cleaned.
{code}
public synchronized boolean isFailedServer(final InetSocketAddress address)
{
if (failedServers.isEmpty()) {
return false;
}
{code}
> Using Map instead of list in FailedServers of RpcClient
> -------------------------------------------------------
>
> Key: HBASE-11707
> URL: https://issues.apache.org/jira/browse/HBASE-11707
> Project: HBase
> Issue Type: Improvement
> Components: Client
> Reporter: Liu Shaohui
> Assignee: Liu Shaohui
> Priority: Minor
> Fix For: 2.0.0
>
> Attachments: HBASE-11707-trunk-v1.diff
>
>
> Currently, FailedServers uses a list to record the black list of servers and
> iterate the list to check if a server is in list. It's not efficient when the
> list is very large. And the list is not thread safe for the add and iteration
> operations.
> RpcClient.java#175
> {code}
> // iterate, looking for the search entry and cleaning expired entries
> Iterator<Pair<Long, String>> it = failedServers.iterator();
> while (it.hasNext()) {
> Pair<Long, String> cur = it.next();
> if (cur.getFirst() < now) {
> it.remove();
> } else {
> if (lookup.equals(cur.getSecond())) {
> return true;
> }
> }
> {code}
> A simple change is to change this list to ConcurrentHashMap.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
