Hadoop Common
  1. Hadoop Common
  2. HADOOP-10490

TestMapFile and TestBloomMapFile leak file descriptors.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.0, 3.0.0
    • Fix Version/s: 2.4.1
    • Component/s: test
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      Multiple tests in TestMapFile and TestBloomMapFile open files but don't close them. On Windows, the leaked file descriptors cause subsequent tests to fail, because file locks are still held while trying to delete the test data directory.

      1. HADOOP-10490.2.patch
        38 kB
        Chris Nauroth
      2. HADOOP-10490.1.patch
        39 kB
        Chris Nauroth

        Activity

        Hide
        Chris Nauroth added a comment -

        This patch ensures that files opened by the tests get closed. Most of the patch is indentation changes caused by introducing try-finally. I successfully ran the tests on both Mac and Windows.

        Show
        Chris Nauroth added a comment - This patch ensures that files opened by the tests get closed. Most of the patch is indentation changes caused by introducing try-finally. I successfully ran the tests on both Mac and Windows.
        Hide
        Chris Nauroth added a comment -

        One more note on this patch: I needed to change the stub CompressionCodec used in the tests to return a non-null value from createOutputStream. This is because the logic inside the close method assumes that the underlying CompressionOutputStream is non-null. Without this change, some of the tests were throwing NullPointerException when I tried to close the files.

        Show
        Chris Nauroth added a comment - One more note on this patch: I needed to change the stub CompressionCodec used in the tests to return a non-null value from createOutputStream . This is because the logic inside the close method assumes that the underlying CompressionOutputStream is non-null. Without this change, some of the tests were throwing NullPointerException when I tried to close the files.
        Hide
        Haohui Mai added a comment -

        The patch looks good. Nit:

        +      writer.close();
        +      writer = null;
        

        Some of these code can be removed. +1 after addressing the comments.

        Show
        Haohui Mai added a comment - The patch looks good. Nit: + writer.close(); + writer = null ; Some of these code can be removed. +1 after addressing the comments.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12639689/HADOOP-10490.1.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. There were no new javadoc 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-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3784//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3784//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/12639689/HADOOP-10490.1.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 . There were no new javadoc 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-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3784//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3784//console This message is automatically generated.
        Hide
        Chris Nauroth added a comment -

        Thanks, Haohui. The redundant setting to null was intended to prevent double-close in the finally block, but it appears these objects are tolerant of double-close. Here is patch v2 removing the redundant setting to null. This version still passes on both Mac and Windows. I'll commit this version after a Jenkins run.

        Show
        Chris Nauroth added a comment - Thanks, Haohui. The redundant setting to null was intended to prevent double-close in the finally block, but it appears these objects are tolerant of double-close. Here is patch v2 removing the redundant setting to null. This version still passes on both Mac and Windows. I'll commit this version after a Jenkins run.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12639728/HADOOP-10490.2.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. There were no new javadoc 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-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3785//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3785//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/12639728/HADOOP-10490.2.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 . There were no new javadoc 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-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3785//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3785//console This message is automatically generated.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #5499 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5499/)
        HADOOP-10490. TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570)

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5499 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5499/ ) HADOOP-10490 . TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Hide
        Chris Nauroth added a comment -

        I committed this to trunk, branch-2 and branch-2.4.

        Show
        Chris Nauroth added a comment - I committed this to trunk, branch-2 and branch-2.4.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #537 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/537/)
        HADOOP-10490. TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570)

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #537 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/537/ ) HADOOP-10490 . TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #1729 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1729/)
        HADOOP-10490. TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570)

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1729 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1729/ ) HADOOP-10490 . TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1754 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1754/)
        HADOOP-10490. TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570)

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1754 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1754/ ) HADOOP-10490 . TestMapFile and TestBloomMapFile leak file descriptors. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586570 ) /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestBloomMapFile.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestMapFile.java

          People

          • Assignee:
            Chris Nauroth
            Reporter:
            Chris Nauroth
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development