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