Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0
    • Fix Version/s: 2.0.3-alpha, trunk-win, 0.23.7
    • Component/s: fs
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      TestHarFileSystemBasics fails on Windows due to invalid HAR URI and file handle leak. We need to change the tests to use valid HAR URIs and fix the file handle leak.

        Issue Links

          Activity

          Hide
          Chris Nauroth added a comment -

          The invalid HAR URI is caused here in TestHarFileSystemBasics:

            private static final Path rootPath = new Path(
                new File(ROOT_PATH).getAbsolutePath() + "/localfs");
          

          On Windows, this creates a path that includes the drive specifier. Later, this gets used to construct a HAR URI for testing, which isn't valid, because it doesn't adhere to the the protocol-host format used by HAR URIs.

          The file handle leak happens in HarFileSystem#HarMetaData#parseMetaData:

               private void parseMetaData() throws IOException {
                FSDataInputStream in = fs.open(masterIndexPath);
          

          This method opens the _masterindex file, but not all code paths guarantee that the file will be closed. For example, parsing an invalid version throws an exception before the _masterindex file gets closed. There is a test that covers this case, so it leaks a file handle when that test runs. On Windows, this ultimately causes tests to fail during their post-test cleanup, because Windows file locking behavior causes the delete to fail.

          Show
          Chris Nauroth added a comment - The invalid HAR URI is caused here in TestHarFileSystemBasics : private static final Path rootPath = new Path( new File(ROOT_PATH).getAbsolutePath() + "/localfs" ); On Windows, this creates a path that includes the drive specifier. Later, this gets used to construct a HAR URI for testing, which isn't valid, because it doesn't adhere to the the protocol-host format used by HAR URIs. The file handle leak happens in HarFileSystem#HarMetaData#parseMetaData : private void parseMetaData() throws IOException { FSDataInputStream in = fs.open(masterIndexPath); This method opens the _masterindex file, but not all code paths guarantee that the file will be closed. For example, parsing an invalid version throws an exception before the _masterindex file gets closed. There is a test that covers this case, so it leaks a file handle when that test runs. On Windows, this ultimately causes tests to fail during their post-test cleanup, because Windows file locking behavior causes the delete to fail.
          Hide
          Chris Nauroth added a comment -

          The attached patch strips the drive specifier from the test working directory path on Windows and ensures that we close file handles that were opened in HarFileSystem#parseMetaData. The patch looks bigger than it really is at first glance because of indentation changes. The actual parsing logic is unchanged.

          With this patch, I ran TestHarFileSystemBasics on Mac and Windows, and it passed on both platforms.

          Show
          Chris Nauroth added a comment - The attached patch strips the drive specifier from the test working directory path on Windows and ensures that we close file handles that were opened in HarFileSystem#parseMetaData . The patch looks bigger than it really is at first glance because of indentation changes. The actual parsing logic is unchanged. With this patch, I ran TestHarFileSystemBasics on Mac and Windows, and it passed on both platforms.
          Hide
          Chris Nauroth added a comment -

          I also meant to mention that this patch can apply to trunk and then merge to branch-trunk-win.

          Show
          Chris Nauroth added a comment - I also meant to mention that this patch can apply to trunk and then merge to branch-trunk-win.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567918/HADOOP-9278.1.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 1 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-common-project/hadoop-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2146//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2146//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/12567918/HADOOP-9278.1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2146//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2146//console This message is automatically generated.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Chris!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Chris!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3325 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3325/)
          HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3325 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3325/ ) HADOOP-9278 . Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1442755 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #119 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/119/)
          HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #119 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/119/ ) HADOOP-9278 . Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1442755 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1308 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1308/)
          HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1308 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1308/ ) HADOOP-9278 . Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1442755 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1336 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1336/)
          HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1336 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1336/ ) HADOOP-9278 . Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. Contributed by Chris Nauroth (Revision 1442755) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1442755 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #518 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/518/)
          HADOOP-9278. Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. (Chris Nauroth via tgraves) (Revision 1443042)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #518 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/518/ ) HADOOP-9278 . Fix the file handle leak in HarMetaData.parseMetaData() in HarFileSystem. (Chris Nauroth via tgraves) (Revision 1443042) Result = SUCCESS tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1443042 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LineReader.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystemBasics.java

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development