[
https://issues.apache.org/jira/browse/HDFS-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16035000#comment-16035000
]
Weiwei Yang commented on HDFS-11887:
------------------------------------
Hi [~msingh]
I thought the purpose of this jira is to fix the xceiverClient leak, but it
seems like a bit deviate now. The original code seems easy to fix, in
{{onRemoval}}
{code}
if (info.hasRefence()) {
// still has reference, do not remove from cache
} else {
// close the client and evict from cache <---- missing this ?
}
{code}
The code you modified seems still works similar as before, but add a lot of
changes.. hmm, do we really need to do that? What else has improved or fixed
other than the resource leak problem? Please elaborate.
And some comments to the code
# {{XceiverClientManager.java}} line 91 {{removalNotification.getValue()}}
returns the instance is going to be removed, lets say clientX. If there is more
references in clientX, clientX will not be closed but it will be removed from
cache. Still has leak?
# {{RefCountedXceiverClient}} is supposed to be internal notion in
{{XceiverClientManager}}, I am not sure why we need to expose this to clients.
I prefer not to change the return value of {{acquireClient}} to make it more
API friendly.
# {{TestXceiverClientManager#testFreeByReference}} line 126 XceiverClient for
containerName1 is evicted, line 130 verifies client1 still works. This seems to
against the idea, is it supposed to keep client1 in cache instead of evicting
it in this case (still has reference)?
Recap the problem, what we want here is
# XceiverClientManager manages a bunch of XceiverClients (per container) in
cache, each XceiverClients can be reused by multiple clients if they want to
access same container.
# XceiverClientManager needs to make sure a XceiverClients won't be removed
from cache as long as there still has client using it (avoid heavy operation
that recreates a connection).
# If a XceiverClients is removed from cache, guarantees it is closed to avoid
resource leak. Let me know if we are on the same page.
I feel we might not be on same page for this, please share your thoughts. Let
me know if I miss anything. Thank you.
> XceiverClientManager should close XceiverClient on eviction from cache
> ----------------------------------------------------------------------
>
> Key: HDFS-11887
> URL: https://issues.apache.org/jira/browse/HDFS-11887
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ozone
> Reporter: Mukul Kumar Singh
> Assignee: Mukul Kumar Singh
> Attachments: HDFS-11887-HDFS-7240.001.patch,
> HDFS-11887-HDFS-7240.002.patch
>
>
> XceiverClientManager doesn't close client on eviction which can leak
> resources.
> {code}
> public XceiverClientManager(Configuration conf) {
> .
> .
> .
> public void onRemoval(
> RemovalNotification<String, XceiverClientWithAccessInfo>
> removalNotification) {
> // If the reference count is not 0, this xceiver client should
> not
> // be evicted, add it back to the cache.
> WithAccessInfo info = removalNotification.getValue();
> if (info.hasRefence()) {
> synchronized (XceiverClientManager.this.openClient) {
> XceiverClientManager.this
> .openClient.put(removalNotification.getKey(), info);
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
