Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1088

Prevent renaming a symlink to its target

    Details

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

      Description

      Here's the HDFS side of HADOOP-6563:

      • Small bug fix for renaming a symlink to it's target
      • Hdfs#getFileBlockLocations needs to use getUriPath() to ensure an absolute path is passed in. See minor change to INode#getPathNames, which now asserts instead of returning null and causing NPEs. This was exposed via calling FileContext#getFileBlockLocations using a symlink.
      • Comment in FSImage around symlinks and image loading
      1. ASF.LICENSE.NOT.GRANTED--hdfs-1088-4.patch
        6 kB
        Eli Collins
      2. ASF.LICENSE.NOT.GRANTED--hdfs-1088-3.patch
        7 kB
        Eli Collins
      3. ASF.LICENSE.NOT.GRANTED--hdfs-1088-2.patch
        7 kB
        Eli Collins
      4. hdfs-1088-1.patch
        3 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Eli Collins added a comment -

          Patch attached.

          Show
          Eli Collins added a comment - Patch attached.
          Hide
          Hadoop QA added a comment -

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

          +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 failed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/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/12441103/hdfs-1088-1.patch against trunk revision 931675. +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 failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/150/console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          There are two variants of rename implementation (one of them is deprecated). Could you please add this check to both of them. Also please throw an exception instead of returning false. I know that the new FSDirectory#unprotectedRenameTo() with rename options is returning false in one of the cases (dst.equals(src)). I think we should change that to throw an exception as well. If you are not doing that in this patch, I will create a bug to track that.

          Show
          Suresh Srinivas added a comment - There are two variants of rename implementation (one of them is deprecated). Could you please add this check to both of them. Also please throw an exception instead of returning false. I know that the new FSDirectory#unprotectedRenameTo() with rename options is returning false in one of the cases (dst.equals(src)). I think we should change that to throw an exception as well. If you are not doing that in this patch, I will create a bug to track that.
          Hide
          Eli Collins added a comment -

          Thanks Suresh. Patch attached addresses all your feedback, including the hdfs side of HADOOP-6703.

          Show
          Eli Collins added a comment - Thanks Suresh. Patch attached addresses all your feedback, including the hdfs side of HADOOP-6703 .
          Hide
          Eli Collins added a comment -

          The tests are all in the base classes which are in the patch for HADOOP-6703.

          Show
          Eli Collins added a comment - The tests are all in the base classes which are in the patch for HADOOP-6703 .
          Hide
          Suresh Srinivas added a comment -

          +1 for the patch. One minor comment. In the exceptions thrown, should we include the name of the file?

          Show
          Suresh Srinivas added a comment - +1 for the patch. One minor comment. In the exceptions thrown, should we include the name of the file?
          Hide
          Eli Collins added a comment -

          Sounds good, updated patch attached.

          Show
          Eli Collins added a comment - Sounds good, updated patch attached.
          Hide
          Eli Collins added a comment -

          Updated corresponding patch over in HADOOP-6703 as well.

          Show
          Eli Collins added a comment - Updated corresponding patch over in HADOOP-6703 as well.
          Hide
          Hadoop QA added a comment -

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

          +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 patch appears to cause tar ant target to fail.

          +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 failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/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/12441993/hdfs-1088-3.patch against trunk revision 934196. +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 patch appears to cause tar ant target to fail. +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 failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/155/console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          +1 for the patch.

          Show
          Suresh Srinivas added a comment - +1 for the patch.
          Hide
          Suresh Srinivas added a comment -

          I am not able to check the test report to see which test failed. Eli, can you please comment on the failed test and if it is unrelated to this patch, when you can access hudson? I will commit the patch after that.

          Show
          Suresh Srinivas added a comment - I am not able to check the test report to see which test failed. Eli, can you please comment on the failed test and if it is unrelated to this patch, when you can access hudson? I will commit the patch after that.
          Hide
          Eli Collins added a comment -

          Attached the same patch but this one doesn't override TestFcHdfsSymlink#unwrapException. That's needed to make the tests in HADOOP-6703 pass. I'll file a separate hdfs jira just to add this override. Splitting the tests (core class and hdfs class) across projects is a pain.

          Show
          Eli Collins added a comment - Attached the same patch but this one doesn't override TestFcHdfsSymlink#unwrapException. That's needed to make the tests in HADOOP-6703 pass. I'll file a separate hdfs jira just to add this override. Splitting the tests (core class and hdfs class) across projects is a pain.
          Hide
          Hadoop QA added a comment -

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

          +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 failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/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/12442020/hdfs-1088-4.patch against trunk revision 934196. +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 failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/317/console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          The test failures are unrelated. The tests are over in HADOOP-6703.

          Show
          Eli Collins added a comment - The test failures are unrelated. The tests are over in HADOOP-6703 .
          Hide
          Suresh Srinivas added a comment -

          I committed the patch to trunk. Thank you Eli.

          Show
          Suresh Srinivas added a comment - I committed the patch to trunk. Thank you Eli.

            People

            • Assignee:
              Eli Collins
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development