Details

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

      Description

      HADOOP-6585 is going to deprecate FileStatus#isDir(). This jira is for replacing all uses of isDir() in HDFS with checks of isDirectory(), isFile(), or isSymlink() as needed.

      1. hdfs-995-1.patch
        12 kB
        Eli Collins
      2. hdfs-995-2.patch
        13 kB
        Eli Collins
      3. hdfs-995-3.patch
        10 kB
        Eli Collins
      4. hdfs-995-4.patch
        13 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Eli Collins added a comment -

          Places Sanjay called out in HDFS-245 that need to be fixed in this jira:

          • FSImage#loadFilesUnderConstruction
                    if (old.isDirectory()) ...
                     add an additional check for isSymlink() -- or better !isFile()
            
          • FSNamesystem#startFileInternal
            } else if (myFile,isDirectory() {
                     ....
            } else if (myFile.isSymlink() {
                       assert(false, cannot reach here ) <-- add
            }
            
          Show
          Eli Collins added a comment - Places Sanjay called out in HDFS-245 that need to be fixed in this jira: FSImage#loadFilesUnderConstruction if (old.isDirectory()) ... add an additional check for isSymlink() -- or better !isFile() FSNamesystem#startFileInternal } else if (myFile,isDirectory() { .... } else if (myFile.isSymlink() { assert ( false , cannot reach here ) <-- add }
          Hide
          Eli Collins added a comment -

          Patch attached. The change is pretty small and straight-forward, mostly updates tests. HDFS rarely uses FileStatus internally. The changes in the previous comment don't apply here because those are for INode rather than FileStatus (and those particular checks don't need to be performed since getFileINode fully resolves the link so the returned INode represents either a file or directory).

          Show
          Eli Collins added a comment - Patch attached. The change is pretty small and straight-forward, mostly updates tests. HDFS rarely uses FileStatus internally. The changes in the previous comment don't apply here because those are for INode rather than FileStatus (and those particular checks don't need to be performed since getFileINode fully resolves the link so the returned INode represents either a file or directory).
          Hide
          Eli Collins added a comment -

          Attached patch merges with trunk.

          Show
          Eli Collins added a comment - Attached patch merges with trunk.
          Hide
          Tom White added a comment -

          +1 This patch has gone stale, can you regenerate please?

          Show
          Tom White added a comment - +1 This patch has gone stale, can you regenerate please?
          Hide
          Eli Collins added a comment -

          Patch attached.

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

          Updated patch attached. Missed some calls to isDir() in TestFileStatus since it got refactored.

          Show
          Eli Collins added a comment - Updated patch attached. Missed some calls to isDir() in TestFileStatus since it got refactored.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 30 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 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/386/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/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/12445728/hdfs-995-4.patch against trunk revision 949333. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 30 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 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/386/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/386/console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          The hdfsproxy failure is HDFS-1164. The TestLargeDirectoryDelete time out is HDFS-816. TestDFSPermission doesn't time out for me locally, don't see a jira for it, can't reproduce it locally.

          Show
          Eli Collins added a comment - The hdfsproxy failure is HDFS-1164 . The TestLargeDirectoryDelete time out is HDFS-816 . TestDFSPermission doesn't time out for me locally, don't see a jira for it, can't reproduce it locally.
          Hide
          Eli Collins added a comment -

          Wrong jira, the TestLargeDirectoryDelete time out is what I saw in HDFS-615. Since this patch just replaces uses of isDir with isDirectory and they have the same implementation that should be a separate issue.

          Show
          Eli Collins added a comment - Wrong jira, the TestLargeDirectoryDelete time out is what I saw in HDFS-615 . Since this patch just replaces uses of isDir with isDirectory and they have the same implementation that should be a separate issue.
          Hide
          Tom White added a comment -

          I've just committed this. Thanks Eli!

          Show
          Tom White added a comment - I've just committed this. Thanks Eli!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #288 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/288/)
          HDFS-995. Replace usage of FileStatus#isDir(). Contributed by Eli Collins.

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #288 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/288/ ) HDFS-995 . Replace usage of FileStatus#isDir(). Contributed by Eli Collins.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development