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-4.patch
        13 kB
        Eli Collins
      2. hdfs-995-3.patch
        10 kB
        Eli Collins
      3. hdfs-995-2.patch
        13 kB
        Eli Collins
      4. hdfs-995-1.patch
        12 kB
        Eli Collins

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          98d 2m 1 Tom White 31/May/10 04:40
          Patch Available Patch Available Resolved Resolved
          15h 17m 1 Tom White 31/May/10 19:57
          Resolved Resolved Closed Closed
          85d 1h 54m 1 Tom White 24/Aug/10 21:51
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          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.
          Tom White made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 0.22.0 [ 12314241 ]
          Resolution Fixed [ 1 ]
          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
          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
          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
          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.
          Tom White made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hadoop Flags [Reviewed]
          Eli Collins made changes -
          Attachment hdfs-995-4.patch [ 12445728 ]
          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.
          Eli Collins made changes -
          Attachment hdfs-995-3.patch [ 12445724 ]
          Hide
          Eli Collins added a comment -

          Patch attached.

          Show
          Eli Collins added a comment - Patch attached.
          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?
          Eli Collins made changes -
          Attachment hdfs-995-2.patch [ 12443364 ]
          Hide
          Eli Collins added a comment -

          Attached patch merges with trunk.

          Show
          Eli Collins added a comment - Attached patch merges with trunk.
          Eli Collins made changes -
          Issue Type Improvement [ 4 ] Bug [ 1 ]
          Fix Version/s 0.21.0 [ 12314046 ]
          Affects Version/s 0.20.3 [ 12314814 ]
          Affects Version/s 0.21.0 [ 12314046 ]
          Affects Version/s 0.22.0 [ 12314241 ]
          Priority Major [ 3 ] Blocker [ 1 ]
          Eli Collins made changes -
          Attachment hdfs-995-1.patch [ 12441235 ]
          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).
          Eli Collins made changes -
          Field Original Value New Value
          Link This issue is blocked by HADOOP-6585 [ HADOOP-6585 ]
          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 }
          Eli Collins created issue -

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development