Hadoop Common
  1. Hadoop Common
  2. HADOOP-6229

Atempt to make a directory under an existing file on LocalFileSystem should throw an Exception.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      This task is sub task of HDFS-303

      "1. When trying to make a directory under an existing file, HDFS throws an IOException while LocalFileSystem doesn't.
      An IOException seems good here. "

      Actually HDFS throws FileNotFoundException in this case (in FSDirectory.mkdirs() ). So I guess we should do the same.

      1. HADOOP-6229-2.patch
        4 kB
        Boris Shkolnik
      2. HADOOP-6229-2.patch
        4 kB
        Boris Shkolnik
      3. HADOOP-6229-1.patch
        3 kB
        Boris Shkolnik
      4. HADOOP-6229.patch
        0.8 kB
        Boris Shkolnik

        Issue Links

          Activity

          Hide
          Tsz Wo Nicholas Sze added a comment -

          > Actually HDFS throws FileNotFoundException in this case (in FSDirectory.mkdirs() ). So I guess we should do the same.
          Does throwing a FileAlreadyExistsException (a new class) make more sense in this case?

          Show
          Tsz Wo Nicholas Sze added a comment - > Actually HDFS throws FileNotFoundException in this case (in FSDirectory.mkdirs() ). So I guess we should do the same. Does throwing a FileAlreadyExistsException (a new class) make more sense in this case?
          Hide
          Konstantin Boudnik added a comment -

          I second Nicholas' suggestion.

          Show
          Konstantin Boudnik added a comment - I second Nicholas' suggestion.
          Hide
          Boris Shkolnik added a comment -

          Took Nikolas' suggestion. Created new class FileAlreadyExestsException.
          Turns out mapred already has such class.
          After this patch is committed I will remove mapred's one and make it use this one (from common).
          Also I will need to change HDFS to use this new exception instead of FileNotFound.

          Show
          Boris Shkolnik added a comment - Took Nikolas' suggestion. Created new class FileAlreadyExestsException. Turns out mapred already has such class. After this patch is committed I will remove mapred's one and make it use this one (from common). Also I will need to change HDFS to use this new exception instead of FileNotFound.
          Hide
          Boris Shkolnik added a comment -

          Ran hadoop-common test, hadoop-hdfs and hadoop-mapreduce with new hadoop-common jar
          Also created a new hadoop-mapreduce jars (with the new core jars) and ran hadoop-hdfs test with them.

          Show
          Boris Shkolnik added a comment - Ran hadoop-common test, hadoop-hdfs and hadoop-mapreduce with new hadoop-common jar Also created a new hadoop-mapreduce jars (with the new core jars) and ran hadoop-hdfs test with them.
          Hide
          Konstantin Boudnik added a comment -

          I believe the original code isn't defensive enough and there's a possibility for NPE to be thrown if Path f happens to be null. I'd add an extra check for this as the very first line of the method.

          Looks good otherwise.

          Show
          Konstantin Boudnik added a comment - I believe the original code isn't defensive enough and there's a possibility for NPE to be thrown if Path f happens to be null . I'd add an extra check for this as the very first line of the method. Looks good otherwise.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          -1 javac. The patch appears to cause tar ant target to fail.

          -1 findbugs. The patch appears to cause Findbugs to fail.

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

          -1 core tests. The patch failed core unit tests.

          -1 contrib tests. The patch failed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/testReport/
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/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/12418453/HADOOP-6229.patch against trunk revision 810714. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to cause Findbugs to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/testReport/ Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/11/console This message is automatically generated.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Forgot to add FileAlreadyExistsException.java?

          Show
          Tsz Wo Nicholas Sze added a comment - Forgot to add FileAlreadyExistsException.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/12418544/HADOOP-6229-1.patch
          against trunk revision 810756.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

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

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/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/12418544/HADOOP-6229-1.patch against trunk revision 810756. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/14/console This message is automatically generated.
          Hide
          Tom White added a comment -

          +1

          This will be tested by the test being introduced in HDFS-303, right?

          Minor nit: we tend not to add serialVersionUID fields. (Any warnings in IDEs should be turned off.)

          Show
          Tom White added a comment - +1 This will be tested by the test being introduced in HDFS-303 , right? Minor nit: we tend not to add serialVersionUID fields. (Any warnings in IDEs should be turned off.)
          Hide
          Boris Shkolnik added a comment -

          removed serialVersionUID.
          Added some unit test in any case for hadoop-common.

          Show
          Boris Shkolnik added a comment - removed serialVersionUID. Added some unit test in any case for hadoop-common.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12418563/HADOOP-6229-2.patch
          against trunk revision 810756.

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

          +1 tests included. The patch appears to include 3 new or modified tests.

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

          -1 javac. The applied patch generated 146 javac compiler warnings (more than the trunk's current 145 warnings).

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/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/12418563/HADOOP-6229-2.patch against trunk revision 810756. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 146 javac compiler warnings (more than the trunk's current 145 warnings). +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/16/console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          +1 patch looks good

          Show
          Konstantin Boudnik added a comment - +1 patch looks good
          Hide
          Tom White added a comment -

          I've just committed this. Thanks Boris!

          Could you update the release note with the new behaviour please?

          Show
          Tom White added a comment - I've just committed this. Thanks Boris! Could you update the release note with the new behaviour please?
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #16 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/16/)
          . Attempt to make a directory under an existing file on LocalFileSystem should throw an Exception. Contributed by Boris Shkolnik.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #16 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/16/ ) . Attempt to make a directory under an existing file on LocalFileSystem should throw an Exception. Contributed by Boris Shkolnik.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #89 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/89/)
          . Attempt to make a directory under an existing file on LocalFileSystem should throw an Exception. Contributed by Boris Shkolnik.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #89 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/89/ ) . Attempt to make a directory under an existing file on LocalFileSystem should throw an Exception. Contributed by Boris Shkolnik.
          Hide
          Robert Chansler added a comment -

          Editorial pass over all release notes prior to publication of 0.21. Bug.

          Show
          Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21. Bug.

            People

            • Assignee:
              Boris Shkolnik
              Reporter:
              Boris Shkolnik
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development