Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3646

LeaseRenewer can hold reference to inactive DFSClient instances forever

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.23.3, 2.0.0-alpha
    • Fix Version/s: 0.23.3, 2.0.2-alpha
    • Component/s: hdfs-client
    • Labels:
      None

      Description

      If LeaseRenewer#closeClient() is not called, LeaseRenewer keeps the reference to a DFSClient instance in dfsclients forever. This prevents DFSClient, LeaseRenewer, conf, etc. from being garbage collected, leading to memory leak.

      LeaseRenewer should remove the reference after some delay, if a DFSClient instance no longer has active streams.

      1. hdfs-3646.patch
        11 kB
        Kihwal Lee
      2. hdfs-3646.patch.txt
        20 kB
        Kihwal Lee
      3. hdfs-3646.patch.txt
        20 kB
        Kihwal Lee
      4. hdfs-3646-branch-23.patch.txt
        21 kB
        Kihwal Lee
      5. hdfs-3646-branch-23.patch.txt
        20 kB
        Kihwal Lee

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #319 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/319/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363368)

        Result = SUCCESS
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363368
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #319 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/319/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363368) Result = SUCCESS daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363368 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Daryn Sharp added a comment -

        I've committed the branch-0.23 patch too. Thanks Kihwal!

        Show
        Daryn Sharp added a comment - I've committed the branch-0.23 patch too. Thanks Kihwal!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12537181/hdfs-3646-branch-23.patch.txt
        against trunk revision .

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2866//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12537181/hdfs-3646-branch-23.patch.txt against trunk revision . -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2866//console This message is automatically generated.
        Hide
        Kihwal Lee added a comment -

        The new patch includes the suggested change.

        Show
        Kihwal Lee added a comment - The new patch includes the suggested change.
        Hide
        Daryn Sharp added a comment -

        Minor comment, can you pull in the closeConnectionToNamenode method instead of directly calling RPC.stopProxy? This will keep the files a bit more in sync with other branches which may help avoid future conflicts. Otherwise it looks good.

        Show
        Daryn Sharp added a comment - Minor comment, can you pull in the closeConnectionToNamenode method instead of directly calling RPC.stopProxy ? This will keep the files a bit more in sync with other branches which may help avoid future conflicts. Otherwise it looks good.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1141 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1141/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170)

        Result = FAILURE
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1141 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1141/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170) Result = FAILURE daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1108 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1108/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170)

        Result = FAILURE
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1108 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1108/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170) Result = FAILURE daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12537122/hdfs-3646-branch-23.patch.txt
        against trunk revision .

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2864//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12537122/hdfs-3646-branch-23.patch.txt against trunk revision . -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2864//console This message is automatically generated.
        Hide
        Kihwal Lee added a comment -

        Sorry I forgot to post the 0.23 patch. I actually had conflicts due to absence of HDFS-3641 and HA when I tested it on 0.23, but somehow forgot about that. I reran all tests in HDFS with the 0.23 patch.

        Show
        Kihwal Lee added a comment - Sorry I forgot to post the 0.23 patch. I actually had conflicts due to absence of HDFS-3641 and HA when I tested it on 0.23, but somehow forgot about that. I reran all tests in HDFS with the 0.23 patch.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2520 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2520/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170)

        Result = FAILURE
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2520 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2520/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170) Result = FAILURE daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2562 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2562/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170)

        Result = SUCCESS
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2562 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2562/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170) Result = SUCCESS daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Daryn Sharp added a comment -

        I've committed to trunk and branch-2, but there are conflicts on branch-0.23.

        Show
        Daryn Sharp added a comment - I've committed to trunk and branch-2, but there are conflicts on branch-0.23.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2497 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2497/)
        HDFS-3646. LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170)

        Result = SUCCESS
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2497 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2497/ ) HDFS-3646 . LeaseRenewer can hold reference to inactive DFSClient instances forever (Kihwal Lee via daryn) (Revision 1363170) Result = SUCCESS daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363170 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSClient.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/LeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSClientAdapter.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestFileAppend4.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLease.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRecovery2.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestLeaseRenewer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestReadWhileWriting.java
        Hide
        Daryn Sharp added a comment -

        +1 Great work! I'll commit tomorrow if nobody else has objections.

        Show
        Daryn Sharp added a comment - +1 Great work! I'll commit tomorrow if nobody else has objections.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12536842/hdfs-3646.patch.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 7 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2844//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2844//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12536842/hdfs-3646.patch.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2844//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2844//console This message is automatically generated.
        Hide
        Kihwal Lee added a comment -

        Since we are now removing the reference to a client when all streams are closed, the renwer's loop exits right away (clientsRunning() is false when there is no more clients left). In order to honor the grace period, I had to move the check and set emptyTime so that the thread is kept alive until isRenewerExpired() returns true.

        The failed test cases don't seem to be related to the patch and I could not reproduce the failures.

        • TestFileAppend4 ran sucessfully, but it timed out because Datanode didn't shutdown.
          012-07-17 01:58:56,034 INFO  datanode.DataNode (DataNode.java:shutdown(1079)) 
          - Waiting for threadgroup to exit, active threads is 1
          
        • TestBlocksWithNotEnoughRacks: MiniDFSCluster exited in the middle of test.

        Regarding null authority, there is a check for nameNodeUri being null, it will throw an IllegalArgumentException. If the uri is not null, but the authority is null, it also won't go far because proxy won't get created properly. Since DFSClient blows up anyway well before a

        {LeaseRenewer}

        is created, the content of variable,

        {authority}

        , being "null" or null does not matter.

        I will upload the updated patch soon.

        Show
        Kihwal Lee added a comment - Since we are now removing the reference to a client when all streams are closed, the renwer's loop exits right away ( clientsRunning() is false when there is no more clients left). In order to honor the grace period, I had to move the check and set emptyTime so that the thread is kept alive until isRenewerExpired() returns true. The failed test cases don't seem to be related to the patch and I could not reproduce the failures. TestFileAppend4 ran sucessfully, but it timed out because Datanode didn't shutdown. 012-07-17 01:58:56,034 INFO datanode.DataNode (DataNode.java:shutdown(1079)) - Waiting for threadgroup to exit, active threads is 1 TestBlocksWithNotEnoughRacks: MiniDFSCluster exited in the middle of test. Regarding null authority, there is a check for nameNodeUri being null, it will throw an IllegalArgumentException. If the uri is not null, but the authority is null, it also won't go far because proxy won't get created properly. Since DFSClient blows up anyway well before a {LeaseRenewer} is created, the content of variable, {authority} , being "null" or null does not matter. I will upload the updated patch soon.
        Hide
        Daryn Sharp added a comment -

        Looks very nice!

        1. In the new comments, "renwer" and "runing" are misspelled
        2. Another comment of "Does this renewer has nothing to renew?" should use "have" instead of "has"
        3. Just a suggestion: Maybe DFS should instantiate the client with its canonical uri instead of its given uri to avoid the authority of "null"
        4. I don't understand this new chunk of code since it looks like emptyTime is internally handled correctly and checked by isRenewerExpired. Is there an edge case this is fixing?
          if (!clientsRunning() && emptyTime == Long.MAX_VALUE) {
            emptyTime = Time.now();
          }
        Show
        Daryn Sharp added a comment - Looks very nice! In the new comments, "renwer" and "runing" are misspelled Another comment of "Does this renewer has nothing to renew?" should use "have" instead of "has" Just a suggestion: Maybe DFS should instantiate the client with its canonical uri instead of its given uri to avoid the authority of "null" I don't understand this new chunk of code since it looks like emptyTime is internally handled correctly and checked by isRenewerExpired . Is there an edge case this is fixing? if (!clientsRunning() && emptyTime == Long .MAX_VALUE) { emptyTime = Time.now(); }
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12536750/hdfs-3646.patch.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 7 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks
        org.apache.hadoop.hdfs.TestFileAppend4

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2837//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2837//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12536750/hdfs-3646.patch.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks org.apache.hadoop.hdfs.TestFileAppend4 +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2837//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2837//console This message is automatically generated.
        Hide
        Kihwal Lee added a comment -

        Dary's suggestion ...

        Sorry Daryn.

        Show
        Kihwal Lee added a comment - Dary's suggestion ... Sorry Daryn.
        Hide
        Kihwal Lee added a comment -

        Followed Dary's suggestion and simplifed the DFSClient side. All affected tests have been updated. The termianl condition of the renewal loop in LeaseRenewer has been changed to make it honor the grace period after dfsclients becomes empty.

        Show
        Kihwal Lee added a comment - Followed Dary's suggestion and simplifed the DFSClient side. All affected tests have been updated. The termianl condition of the renewal loop in LeaseRenewer has been changed to make it honor the grace period after dfsclients becomes empty.
        Hide
        Kihwal Lee added a comment -

        LeaseRenewer#getInstance() doesn't seem heavy. The only danger will be getting a different (newer) instance when trying to free something, but that shouldn't happen since it means all streams were closed before. when it happens the client was removed from renewer's client list, so there will be no reference leak. So I think we can safely remove the class level reference holder for LeaseRenewer as you said and use getInstance() method.

        I will make changes accordingly and also address the other breakages reported in the test-patch.

        Show
        Kihwal Lee added a comment - LeaseRenewer#getInstance() doesn't seem heavy. The only danger will be getting a different (newer) instance when trying to free something, but that shouldn't happen since it means all streams were closed before. when it happens the client was removed from renewer's client list, so there will be no reference leak. So I think we can safely remove the class level reference holder for LeaseRenewer as you said and use getInstance() method. I will make changes accordingly and also address the other breakages reported in the test-patch.
        Hide
        Daryn Sharp added a comment -

        In DFSClient#beginFileLease, should leaserenewer.put(src, out, this) be in the synchronized block?

        Instead of trying to break the hard ref from client to renewer when streams aren't open, would it maybe make sense to remove the hard reference entirely? Maybe the client could use a LeaseRenewer.getInstance(String, UserGroupInformation) (note the client isn't passed) when it needs to invoke methods on the renewer? I haven't fully thought it through...

        Show
        Daryn Sharp added a comment - In DFSClient#beginFileLease , should leaserenewer.put(src, out, this) be in the synchronized block? Instead of trying to break the hard ref from client to renewer when streams aren't open, would it maybe make sense to remove the hard reference entirely? Maybe the client could use a LeaseRenewer.getInstance(String, UserGroupInformation) (note the client isn't passed) when it needs to invoke methods on the renewer? I haven't fully thought it through...
        Hide
        Kihwal Lee added a comment -

        This patch will allow LeaseRenewer and DFSClient to be GC'ed when all streams are closed by users. Oozie once reported an OOM due to this problem. I looked at the heap dump and there were a large number of trios: DFSClient, JobConf and the array entry in LeaseRenewer. There were no leaked streams/sockets, or FileSystem cache bloat. Had they given up references to others, they would have been GC'ed. (there was no other external reference to them)

        What Daryn mentioned will solve a diffetent problem, which can happen when streams are lost without being closed. Daryn has some ideas, so he will probably file a separate jira once its feasibility checks out.

        Show
        Kihwal Lee added a comment - This patch will allow LeaseRenewer and DFSClient to be GC'ed when all streams are closed by users. Oozie once reported an OOM due to this problem. I looked at the heap dump and there were a large number of trios: DFSClient, JobConf and the array entry in LeaseRenewer. There were no leaked streams/sockets, or FileSystem cache bloat. Had they given up references to others, they would have been GC'ed. (there was no other external reference to them) What Daryn mentioned will solve a diffetent problem, which can happen when streams are lost without being closed. Daryn has some ideas, so he will probably file a separate jira once its feasibility checks out.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12536477/hdfs-3646.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer
        org.apache.hadoop.hdfs.TestDistributedFileSystem

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2819//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2819//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12536477/hdfs-3646.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.tools.offlineEditsViewer.TestOfflineEditsViewer org.apache.hadoop.hdfs.TestDistributedFileSystem +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2819//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2819//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        Hey Kihwal. I'm looking at this patch. Does this intend to fix the entire problem as described? Or is it just a step along the way? Haven't thought deeply enough about it to be sure, yet.

        Show
        Todd Lipcon added a comment - Hey Kihwal. I'm looking at this patch. Does this intend to fix the entire problem as described? Or is it just a step along the way? Haven't thought deeply enough about it to be sure, yet.
        Hide
        Kihwal Lee added a comment -

        The goal of this patch is to remove unnecessary references from both DFSClient and LeaseRenewer whenever possible.

        Show
        Kihwal Lee added a comment - The goal of this patch is to remove unnecessary references from both DFSClient and LeaseRenewer whenever possible.
        Hide
        Daryn Sharp added a comment -

        IMO, this will a leak from application side, since there is a bug in closing the streams from app.

        Agreed, but it can have pretty severe consequences that aren't easily detected unless explicitly hunting for leaks. It makes me uneasy that an out of scope fs stream can cause a massive leak of heavy objects, threads, and tie up sockets that may exhaust fds and/or memory for long running processes. Emitting an angry log error for lost & unclosed streams may be more beneficial.

        I don't think a finalizer on the fs will work. If I do in = path.getFileSystem(conf).open(...), the fs might get garbage collected but we certainly don't want its finalizer to shoot the dfs client that is still holding open a stream. Maybe a finalizer on the dfs client, but in any case, the circular hard references need to be broken somehow.

        Show
        Daryn Sharp added a comment - IMO, this will a leak from application side, since there is a bug in closing the streams from app. Agreed, but it can have pretty severe consequences that aren't easily detected unless explicitly hunting for leaks. It makes me uneasy that an out of scope fs stream can cause a massive leak of heavy objects, threads, and tie up sockets that may exhaust fds and/or memory for long running processes. Emitting an angry log error for lost & unclosed streams may be more beneficial. I don't think a finalizer on the fs will work. If I do in = path.getFileSystem(conf).open(...) , the fs might get garbage collected but we certainly don't want its finalizer to shoot the dfs client that is still holding open a stream. Maybe a finalizer on the dfs client, but in any case, the circular hard references need to be broken somehow.
        Hide
        Kihwal Lee added a comment -

        .... the lost stream will remain open as long as the client is open.

        I think Daryn is bringing up the issue because its solution also take care of this jira. If we have a finializer for FileSystem, we could have it call close(), then everything will go away.

        But short of automatic cleaning, this issue still remains. Currently DFSClient won't get garbage collected even if lost streams are automatically closed. I think we should still fix it, even if we eventually implement automatic clean-up.

        Show
        Kihwal Lee added a comment - .... the lost stream will remain open as long as the client is open. I think Daryn is bringing up the issue because its solution also take care of this jira. If we have a finializer for FileSystem, we could have it call close(), then everything will go away. But short of automatic cleaning, this issue still remains. Currently DFSClient won't get garbage collected even if lost streams are automatically closed. I think we should still fix it, even if we eventually implement automatic clean-up.
        Hide
        Uma Maheswara Rao G added a comment -

        Unfortunately accidents do happen and streams don't always get closed. Since DFSClient has a hard reference to the stream, the lost stream will remain open as long as the client is open.

        IMO, this will a leak from application side, since there is a bug in closing the streams from app.

        Show
        Uma Maheswara Rao G added a comment - Unfortunately accidents do happen and streams don't always get closed. Since DFSClient has a hard reference to the stream, the lost stream will remain open as long as the client is open. IMO, this will a leak from application side, since there is a bug in closing the streams from app.
        Hide
        Daryn Sharp added a comment -

        I agree with everything you said if a client code is still holding a reference to the stream. Unfortunately accidents do happen and streams don't always get closed. Since DFSClient has a hard reference to the stream, the lost stream will remain open as long as the client is open. In turn, the lost stream will prevent the lease renewer from removing the client when all other streams are closed.

        Show
        Daryn Sharp added a comment - I agree with everything you said if a client code is still holding a reference to the stream. Unfortunately accidents do happen and streams don't always get closed. Since DFSClient has a hard reference to the stream, the lost stream will remain open as long as the client is open. In turn, the lost stream will prevent the lease renewer from removing the client when all other streams are closed.
        Hide
        Uma Maheswara Rao G added a comment -

        There will be caveats, such as the leak will still occur if client code doesn't explicitly close all streams.

        If client code doesn't close the file, dfsClient object should be there and lease renewal should happen as file is in open state. At that time keeping the reference in LeaseRenewer will not be a leak. Please correct me, if I understood your point wrongly.

        Show
        Uma Maheswara Rao G added a comment - There will be caveats, such as the leak will still occur if client code doesn't explicitly close all streams. If client code doesn't close the file, dfsClient object should be there and lease renewal should happen as file is in open state. At that time keeping the reference in LeaseRenewer will not be a leak. Please correct me, if I understood your point wrongly.
        Hide
        Daryn Sharp added a comment -

        There will be caveats, such as the leak will still occur if client code doesn't explicitly close all streams. I'm not sure how you can tell if there are no more references since DFSClient holds references to all open streams. Maybe weak references to the streams could be used?

        Show
        Daryn Sharp added a comment - There will be caveats, such as the leak will still occur if client code doesn't explicitly close all streams. I'm not sure how you can tell if there are no more references since DFSClient holds references to all open streams. Maybe weak references to the streams could be used?
        Hide
        Kihwal Lee added a comment -

        Thanks Uma. That makes sense.

        Show
        Kihwal Lee added a comment - Thanks Uma. That makes sense.
        Hide
        Uma Maheswara Rao G added a comment -

        Kihwal, Thanks for filing the JIRA.
        I have seen this. One possible option to fix this issue is:
        Actually lease renewer required for the opened files. So, while opening the file it can add the renewer if there is no client present in Renewer's list of clients.
        So, file close can remove the dfsCLinet instance completely if there is no filesBeingWritten with that client. Means that, if there is no open files with a particular DFSClient, then that clinet will not be there with renewer. If the same DFSClient wants to open new file, it will take care of adding client to renewer.
        How does this sounds to you?

        Show
        Uma Maheswara Rao G added a comment - Kihwal, Thanks for filing the JIRA. I have seen this. One possible option to fix this issue is: Actually lease renewer required for the opened files. So, while opening the file it can add the renewer if there is no client present in Renewer's list of clients. So, file close can remove the dfsCLinet instance completely if there is no filesBeingWritten with that client. Means that, if there is no open files with a particular DFSClient, then that clinet will not be there with renewer. If the same DFSClient wants to open new file, it will take care of adding client to renewer. How does this sounds to you?

          People

          • Assignee:
            Kihwal Lee
            Reporter:
            Kihwal Lee
          • Votes:
            0 Vote for this issue
            Watchers:
            13 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development