Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.21.0
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      This issue adds Iterator<FileStatus> listStatus(Path) to FileContext, moves FileStatus[] listStatus(Path) to FileContext#Util, and adds Iterator<FileStatus> listStatusItor(Path) to AbstractFileSystem which provides a default implementation by using FileStatus[] listStatus(Path).
      Show
      This issue adds Iterator<FileStatus> listStatus(Path) to FileContext, moves FileStatus[] listStatus(Path) to FileContext#Util, and adds Iterator<FileStatus> listStatusItor(Path) to AbstractFileSystem which provides a default implementation by using FileStatus[] listStatus(Path).

      Description

      Add a method Iterator<FileStatus> listStatus(Path), which allows HDFS client not to have the whole listing in the memory, benefit more from the iterative listing added in HDFS-985. Move the current FileStatus[] listStatus(Path) to be a utility method.

      1. listStatusItor2.patch
        14 kB
        Hairong Kuang
      2. listStatusItor1.patch
        13 kB
        Hairong Kuang
      3. listStatusItor.patch
        13 kB
        Hairong Kuang

        Issue Links

          Activity

          Hide
          Hairong Kuang added a comment -

          This patch:

          1. add Iterator<FileStatus> listStatus(Path) to FileContext;
          2. move FileStatus[] listStatus(Path) to FileContext#Util;
          3. add Iterator<FileStatus> listStatusItor(Path) to AbstractFileSystem which provides a default implementation by using FileStatus[] listStatus(Path);
          4. add unit test to test Iterator<FileStatus> listStatus(Path).
          Show
          Hairong Kuang added a comment - This patch: add Iterator<FileStatus> listStatus(Path) to FileContext; move FileStatus[] listStatus(Path) to FileContext#Util; add Iterator<FileStatus> listStatusItor(Path) to AbstractFileSystem which provides a default implementation by using FileStatus[] listStatus(Path); add unit test to test Iterator<FileStatus> listStatus(Path).
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12441211/listStatusItor.patch
          against trunk revision 932115.

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 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 passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/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/12441211/listStatusItor.patch against trunk revision 932115. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/455/console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          Hairong, this is a good change.

          In order to simplify transition to FileContext, should we retain the old FileContext.listStatus() method as it is and add iterative list status as listStatusIterator()? It also captures the semantics quite well.

          Minor:

          1. FileContext.listStatus() that returns iterator - add information about throwing UnsupportedOperationException and NoSuchElementException in the RuntimeExceptions section on attempt to delete an entry from iterator.
          2. FileContext.listStatus() that retuns array - has invalid description in @return. Also add a note to use the itertor version if the need is to iterate over the list, with a @link to the other method
          3. AFS.listStatusItertor() - method javadoc should point @link to the right method in FileContext.
          4. AFS.listStatus() - method javadoc should point @link to the right method in FileContext. Current link is invalid and is missing # prefix to listStatus().
          Show
          Suresh Srinivas added a comment - Hairong, this is a good change. In order to simplify transition to FileContext, should we retain the old FileContext.listStatus() method as it is and add iterative list status as listStatusIterator()? It also captures the semantics quite well. Minor: FileContext.listStatus() that returns iterator - add information about throwing UnsupportedOperationException and NoSuchElementException in the RuntimeExceptions section on attempt to delete an entry from iterator. FileContext.listStatus() that retuns array - has invalid description in @return. Also add a note to use the itertor version if the need is to iterate over the list, with a @link to the other method AFS.listStatusItertor() - method javadoc should point @link to the right method in FileContext. AFS.listStatus() - method javadoc should point @link to the right method in FileContext. Current link is invalid and is missing # prefix to listStatus().
          Hide
          Eli Collins added a comment -

          Hey Hairong,

          The patch looks good to me. Only one minor edit in addition to Suresh's comments: FileContext#listStatus will never throw UnresolvedLinkException so you can remove that from the throws clause and javadoc.

          Thanks,
          Eli

          Show
          Eli Collins added a comment - Hey Hairong, The patch looks good to me. Only one minor edit in addition to Suresh's comments: FileContext#listStatus will never throw UnresolvedLinkException so you can remove that from the throws clause and javadoc. Thanks, Eli
          Hide
          Hairong Kuang added a comment -

          Thank Suresh and Eli for review my patch. Those are good feedback. This patch addresses most of the comments except for
          > FileContext.listStatus() that returns iterator - add information about throwing UnsupportedOperationException and NoSuchElementException in the RuntimeExceptions section on attempt to delete an entry from iterator.

          Those exceptions are not thrown in listStatus method. Those may be thrown when iterating happens.

          > AFS.listStatusItertor() - method javadoc should point @link to the right method in FileContext.
          It points to the right method. It happens that in FileContext the method that returns an iterator is named as listStatus.

          Show
          Hairong Kuang added a comment - Thank Suresh and Eli for review my patch. Those are good feedback. This patch addresses most of the comments except for > FileContext.listStatus() that returns iterator - add information about throwing UnsupportedOperationException and NoSuchElementException in the RuntimeExceptions section on attempt to delete an entry from iterator. Those exceptions are not thrown in listStatus method. Those may be thrown when iterating happens. > AFS.listStatusItertor() - method javadoc should point @link to the right method in FileContext. It points to the right method. It happens that in FileContext the method that returns an iterator is named as listStatus.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442712/listStatusItor1.patch
          against trunk revision 937183.

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

          +1 tests included. The patch appears to include 9 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 passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/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/12442712/listStatusItor1.patch against trunk revision 937183. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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 passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/476/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
          Hairong Kuang added a comment -

          This patch in addition made a change to TestFilterFs to fix the failed test.

          Show
          Hairong Kuang added a comment - This patch in addition made a change to TestFilterFs to fix the failed test.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12442734/listStatusItor2.patch
          against trunk revision 937577.

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

          +1 tests included. The patch appears to include 12 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 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/477/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/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/12442734/listStatusItor2.patch against trunk revision 937577. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 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 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/477/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/477/console This message is automatically generated.
          Hide
          Hairong Kuang added a comment -

          I've committed this!

          Show
          Hairong Kuang added a comment - I've committed this!

            People

            • Assignee:
              Hairong Kuang
              Reporter:
              Hairong Kuang
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development