hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mukul Kumar Singh (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HDFS-11887) XceiverClientManager should close XceiverClient on eviction from cache
Date Sun, 04 Jun 2017 08:18:04 GMT

    [ https://issues.apache.org/jira/browse/HDFS-11887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16036210#comment-16036210
] 

Mukul Kumar Singh edited comment on HDFS-11887 at 6/4/17 8:17 AM:
------------------------------------------------------------------

Thanks for taking a look at the patch and for your comments [~cheersyang],

In the example given in your comments, 

Cleanup will be called as part of releaseClient when *clientA* is released by the consumer
and the reference count drops to 0. (the client has already been evicted from cache)

The code flow will be something like this.
{releaseClient} --> {decrementReference} --> {cleanup} --> {close connection}

Please note that cleanup is triggered as part of both eviction as well as releaseClient.
The connection is closed only when both of the conditions are satisfied, i.e. client has been
evicted from cache as well as when there are no users of the client (refcount is 0).
This will ensure that if there is an operation which has acquired a client before it was evicted
from cache should be able to keep using the acquired connection.

 [~vagarychen] had pointed out this in one of the earlier comments
{code}
So I wonder what happens if there is a long-lasting open connection, it expires in the cache
but it is used by someone. Will it be problematic if we close the connection here?
{code}

Sure, we can think about implementing a cache by scheduling an eviction thread but that would
involve dealing with high and low watermarks in cache and scheduling of eviction thread, hence
I am inclined on fixing the issue with the current implementation. Please let me know of your
thoughts on this.


was (Author: msingh):
Thanks for taking a look at the patch and for your comments [~cheersyang],

In the example given in your comments, 

Cleanup will be called as part of releaseClient when *clientA* is released by the consumer
and the reference count drops to 0. (the client has already been evicted from cache)

The code flow will be something like this.
*releaseClient --> decrementReference --> cleanup--> close connection*

Please note that cleanup is triggered as part of both eviction as well as releaseClient.
The connection is closed only when both of the conditions are satisfied, i.e. client has been
evicted from cache as well as when there are no users of the client (refcount is 0).
This will ensure that if there is an operation which has acquired a client before it was evicted
from cache should be able to keep using the acquired connection.

 [~vagarychen] had pointed out this in one of the earlier comments
{code}
So I wonder what happens if there is a long-lasting open connection, it expires in the cache
but it is used by someone. Will it be problematic if we close the connection here?
{code}

Sure, we can think about implementing a cache by scheduling an eviction thread but that would
involve dealing with high and low watermarks in cache and scheduling of eviction thread, hence
I am inclined on fixing the issue with the current implementation. Please let me know of your
thoughts on this.

> 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}
> Also a stack overflow can be triggered because of putting the element back in the cache
on eviction.
> {code}
>                 synchronized (XceiverClientManager.this.openClient) {
>                   XceiverClientManager.this
>                       .openClient.put(removalNotification.getKey(), info);
>                 }
> {code}
> This bug will try to fix both of these cases.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message