[
https://issues.apache.org/jira/browse/HADOOP-12475?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Walter Su updated HADOOP-12475:
-------------------------------
Attachment: HADOOP-12475.02.patch
1. Connection leak means an opened connection which forgets to close. It
doesn't happen here. Connection is opened at {{setupIOstreams}}. And it will be
closed even it's not in the cache.
2. It's true a race condition can cause creating multiple connections for the
same remoteId. I replace it with {{putIfAbsent}}. Thanks [~Apache9], [~sjlee0]
3. There is another race condition about removing. It's caused by HADOOP-11772.
Before HADOOP-11772, Client used {{HashTable}} for caching. The removing logic
is
{code}
1175 synchronized (connections) {
1176 if (connections.get(remoteId) == this) {
1177 connections.remove(remoteId);
1178 }
1179 }
{code}
It bothers me a while why a thread-safe HashTable need {{synchronized}}. Then I
know this connection is closed, should be removed. But other thread could have
already known this closedConnection, and replace it with a new connection.
I use conditional remove {{ConcurrentHashMap.remove(key,value)}} to address
this.
I think before HADOOP-11772, {{synchronized (connections)}} (which locks the
whole table and HashTable also locks whole table) is the reason why the
invokers choke up.
Uploaded 02 patch.
> Replace guava Cache with ConcurrentHashMap for caching Connection in ipc
> Client
> -------------------------------------------------------------------------------
>
> Key: HADOOP-12475
> URL: https://issues.apache.org/jira/browse/HADOOP-12475
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: conf, io, ipc
> Reporter: Walter Su
> Assignee: Walter Su
> Attachments: HADOOP-12475.01.patch, HADOOP-12475.02.patch
>
>
> quote [~daryn] from HADOOP-11772:
> {quote}
> CacheBuilder is obscenely expensive for concurrent map, and it requires
> generating unnecessary garbage even just to look up a key. Replace it with
> ConcurrentHashMap.
> I identified this issue that impaired my own perf testing under load. The
> slowdown isn't just the sync. It's the expensive of Connection's ctor
> stalling other connections. The expensive of ConnectionId#equals causes
> delays. Synch'ing on connections causes unfair contention unlike a sync'ed
> method. Concurrency simply hides this.
> {quote}
> BTW, guava Cache is heavyweight. Per local test, ConcurrentHashMap has better
> overal performance.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
