Details

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

      Description

      FSDirectory's getFullPathName method is flawed. Given a list of inodes, it starts at index 1 instead of 0 (based on the assumption that inode[0] is always the root inode) and then builds the string with "/"+inode[i]. This means the empty string is returned for the root, or when requesting the full path of the parent dir for top level items.

      In addition, it's not guaranteed that the list of inodes starts with the root inode. The inode lookup routine will only fill the inode array with the last n-many inodes of a path if the array is smaller than the path. In these cases, getFullPathName will skip the first component of the relative path, and then assume the second component starts at the root. ex. "a/b/c" becomes "/b/c".

      There are a few places in the code where the issue was hacked around by assuming that a 0-length path meant a hardcoded "/" instead of Path.SEPARATOR.

      1. HDFS-1760-2.patch
        3 kB
        Daryn Sharp
      2. HDFS-1760.patch
        3 kB
        Daryn Sharp

        Activity

        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12473951/HDFS-1760.patch
        against trunk revision 1082263.

        +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 does not increase the total number of javac compiler warnings.

        +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 failed these core unit tests:
        org.apache.hadoop.hdfs.server.datanode.TestBlockReport
        org.apache.hadoop.hdfs.server.datanode.TestTransferRbw

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//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/12473951/HDFS-1760.patch against trunk revision 1082263. +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 does not increase the total number of javac compiler warnings. +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 failed these core unit tests: org.apache.hadoop.hdfs.server.datanode.TestBlockReport org.apache.hadoop.hdfs.server.datanode.TestTransferRbw -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/267//console This message is automatically generated.
        Hide
        Daryn Sharp added a comment -

        The tests that failed appear unrelated to this change.

        These tests have been sporadically failing in prior builds:
        org.apache.hadoop.hdfs.server.datanode.TestTransferRbw.testTransferRbw
        org.apache.hadoop.hdfs.server.datanode.TestBlockReport.blockReport_08

        These tests have been consistently failing for a long time.
        org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermit
        org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermitQualified

        Requesting approval despite the test failures.

        Show
        Daryn Sharp added a comment - The tests that failed appear unrelated to this change. These tests have been sporadically failing in prior builds: org.apache.hadoop.hdfs.server.datanode.TestTransferRbw.testTransferRbw org.apache.hadoop.hdfs.server.datanode.TestBlockReport.blockReport_08 These tests have been consistently failing for a long time. org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermit org.apache.hadoop.hdfsproxy.TestAuthorizationFilter.testPathPermitQualified Requesting approval despite the test failures.
        Hide
        Daryn Sharp added a comment -

        Good catch. Removed duplicate name tag.

        Show
        Daryn Sharp added a comment - Good catch. Removed duplicate name tag.
        Hide
        Daryn Sharp added a comment -

        Ignore last comment. Wrong jira.

        Show
        Daryn Sharp added a comment - Ignore last comment. Wrong jira.
        Hide
        John George added a comment -

        Good catch Daryn.

        Line 236 in INode.java - are the checks "p_node == null" and "isRoot()" the same? If so, can they be made simpler.

        Show
        John George added a comment - Good catch Daryn. Line 236 in INode.java - are the checks "p_node == null" and "isRoot()" the same? If so, can they be made simpler.
        Hide
        Daryn Sharp added a comment -

        Simplified the requested logic. There is a subtle difference between isRoot() and p_node == null. The parent node will happen to be null for the root, but will also be null for an inode not yet linked into the directory structure.

        Show
        Daryn Sharp added a comment - Simplified the requested logic. There is a subtle difference between isRoot() and p_node == null. The parent node will happen to be null for the root, but will also be null for an inode not yet linked into the directory structure.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12476057/HDFS-1760-2.patch
        against trunk revision 1091131.

        +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 does not increase the total number of javac compiler warnings.

        +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 failed these core unit tests:
        org.apache.hadoop.hdfs.TestFileConcurrentReader

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

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//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/12476057/HDFS-1760-2.patch against trunk revision 1091131. +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 does not increase the total number of javac compiler warnings. +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 failed these core unit tests: org.apache.hadoop.hdfs.TestFileConcurrentReader -1 contrib tests. The patch failed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HDFS-Build/344//console This message is automatically generated.
        Hide
        John George added a comment -

        +1.
        looks good to me.

        Show
        John George added a comment - +1. looks good to me.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #590 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/590/)
        HDFS-1760. In FSDirectory.getFullPathName(..), it is better to return "/" for root directory instead of an empty string. Contributed by Daryn Sharp

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #590 (See https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/590/ ) HDFS-1760 . In FSDirectory.getFullPathName(..), it is better to return "/" for root directory instead of an empty string. Contributed by Daryn Sharp
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this. Thanks, Daryn!

        Also thanks John for reviewing it.

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Daryn! Also thanks John for reviewing it.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #643 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/643/)

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #643 (See https://builds.apache.org/hudson/job/Hadoop-Hdfs-trunk/643/ )

          People

          • Assignee:
            Daryn Sharp
            Reporter:
            Daryn Sharp
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development