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

          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          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
          Thomas Graves made changes -
          Fix Version/s 0.23.7 [ 12323956 ]
          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-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-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-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
          Tsz Wo Nicholas Sze made changes -
          Summary TestHarFileSystemBasics fails on Windows due to invalid HAR URI and file handle leak HarFileSystem may leak file handle
          Description We need to change the tests to use valid HAR URIs and fix the file handle leak. 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.
          Tsz Wo Nicholas Sze made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 2.0.3-alpha [ 12323273 ]
          Fix Version/s trunk-win [ 12323306 ]
          Resolution Fixed [ 1 ]
          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!
          Tsz Wo Nicholas Sze made changes -
          Hadoop Flags Reviewed [ 10343 ]
          Component/s test [ 12311440 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Chris Nauroth made changes -
          Link This issue is part of HADOOP-8562 [ HADOOP-8562 ]
          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.
          Chris Nauroth made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          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.
          Chris Nauroth made changes -
          Field Original Value New Value
          Attachment HADOOP-9278.1.patch [ 12567918 ]
          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 -

          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.
          Chris Nauroth created issue -

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development