HADOOP-6900. Make the iterator returned by FileSystem#listLocatedStatus to throw IOException rather than RuntimeException when there is an IO error fetching the next file. Contributed by Hairong Kuang.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@984301 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -111,7 +111,11 @@ Trunk (unreleased changes)
     HADOOP-6862. Adds api to add/remove user and group to AccessControlList
     (amareshwari)
 
-    HADOOP6911. doc update for DelegationTokenFetcher (boryas)
+    HADOOP-6911. doc update for DelegationTokenFetcher (boryas)
+
+    HADOOP-6900. Make the iterator returned by FileSystem#listLocatedStatus to 
+    throw IOException rather than RuntimeException when there is an IO error
+    fetching the next file. (hairong)
 
   OPTIMIZATIONS
 

src/java/org/apache/hadoop/fs/AbstractFileSystem.java

@@ -619,7 +619,7 @@ public abstract class AbstractFileSystem {
       }
       // Delete the destination that is a file or an empty directory
       if (dstStatus.isDirectory()) {
-        Iterator<FileStatus> list = listStatusIterator(dst);
+        RemoteIterator<FileStatus> list = listStatusIterator(dst);
         if (list != null && list.hasNext()) {
           throw new IOException(
               "Rename cannot overwrite non empty destination directory " + dst);
@@ -757,10 +757,10 @@ public abstract class AbstractFileSystem {
    * {@link FileContext#listStatus(Path)} except that Path f must be for this
    * file system.
    */
-  protected Iterator<FileStatus> listStatusIterator(final Path f)
+  protected RemoteIterator<FileStatus> listStatusIterator(final Path f)
       throws AccessControlException, FileNotFoundException,
       UnresolvedLinkException, IOException {
-    return new Iterator<FileStatus>() {
+    return new RemoteIterator<FileStatus>() {
       private int i = 0;
       private FileStatus[] statusList = listStatus(f);
       
@@ -776,11 +776,6 @@ public abstract class AbstractFileSystem {
         }
         return statusList[i++];
       }
-      
-      @Override
-      public void remove() {
-        throw new UnsupportedOperationException("Remove is not supported");
-      }
     };
   }
 
@@ -789,52 +784,29 @@ public abstract class AbstractFileSystem {
    * {@link FileContext#listLocatedStatus(Path)} except that Path f 
    * must be for this file system.
    */
-  protected Iterator<LocatedFileStatus> listLocatedStatus(final Path f)
+  protected RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f)
       throws AccessControlException, FileNotFoundException,
       UnresolvedLinkException, IOException {
-    return new Iterator<LocatedFileStatus>() {
+    return new RemoteIterator<LocatedFileStatus>() {
-      private Iterator<FileStatus> itor = listStatusIterator(f);
+      private RemoteIterator<FileStatus> itor = listStatusIterator(f);
       
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDog} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       */
       @Override
-      public boolean hasNext() {
+      public boolean hasNext() throws IOException {
         return itor.hasNext();
       }
       
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDoc} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       *  @exception {@inheritDoc}
-       */
       @Override
-      public LocatedFileStatus next() {
+      public LocatedFileStatus next() throws IOException {
         if (!hasNext()) {
           throw new NoSuchElementException("No more entry in " + f);
         }
         FileStatus result = itor.next();
-        try {
+        BlockLocation[] locs = null;
-          
+        if (result.isFile()) {
-          BlockLocation[] locs = null;
+          locs = getFileBlockLocations(
-          if (result.isFile()) {
-            locs = getFileBlockLocations(
               result.getPath(), 0, result.getLen());
-          }
-          return new LocatedFileStatus(result, locs);
-        } catch (IOException ioe) {
-          throw (RuntimeException)new RuntimeException().initCause(ioe);
         }
-      }
+        return new LocatedFileStatus(result, locs);
-      
-      @Override
-      public void remove() {
-        throw new UnsupportedOperationException("Remove is not supported");
       }
     };
   }

src/java/org/apache/hadoop/fs/ChecksumFileSystem.java

@@ -504,7 +504,7 @@ public abstract class ChecksumFileSystem extends FilterFileSystem {
    * @throws IOException
    */
   @Override
-  public Iterator<LocatedFileStatus> listLocatedStatus(Path f)
+  public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
   throws IOException {
     return fs.listLocatedStatus(f, DEFAULT_FILTER);
   }

src/java/org/apache/hadoop/fs/FileContext.java

@@ -27,10 +27,8 @@ import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.IdentityHashMap;
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.Stack;
 import java.util.TreeSet;
@@ -1275,12 +1273,13 @@ public final class FileContext {
    * @throws UnexpectedServerException If server implementation throws 
    *           undeclared exception to RPC server
    */
-  public Iterator<FileStatus> listStatus(final Path f) throws
+  public RemoteIterator<FileStatus> listStatus(final Path f) throws
       AccessControlException, FileNotFoundException,
       UnsupportedFileSystemException, IOException {
     final Path absF = fixRelativePart(f);
-    return new FSLinkResolver<Iterator<FileStatus>>() {
+    return new FSLinkResolver<RemoteIterator<FileStatus>>() {
-      public Iterator<FileStatus> next(final AbstractFileSystem fs, final Path p) 
+      public RemoteIterator<FileStatus> next(
+          final AbstractFileSystem fs, final Path p) 
         throws IOException, UnresolvedLinkException {
         return fs.listStatusIterator(p);
       }
@@ -1314,12 +1313,13 @@ public final class FileContext {
    * @throws UnexpectedServerException If server implementation throws 
    *           undeclared exception to RPC server
    */
-  public Iterator<LocatedFileStatus> listLocatedStatus(final Path f) throws
+  public RemoteIterator<LocatedFileStatus> listLocatedStatus(
+      final Path f) throws
       AccessControlException, FileNotFoundException,
       UnsupportedFileSystemException, IOException {
     final Path absF = fixRelativePart(f);
-    return new FSLinkResolver<Iterator<LocatedFileStatus>>() {
+    return new FSLinkResolver<RemoteIterator<LocatedFileStatus>>() {
-      public Iterator<LocatedFileStatus> next(
+      public RemoteIterator<LocatedFileStatus> next(
           final AbstractFileSystem fs, final Path p) 
         throws IOException, UnresolvedLinkException {
         return fs.listLocatedStatus(p);
@@ -1466,7 +1466,8 @@ public final class FileContext {
         return new ContentSummary(status.getLen(), 1, 0);
       }
       long[] summary = {0, 0, 1};
-      Iterator<FileStatus> statusIterator = FileContext.this.listStatus(f);
+      RemoteIterator<FileStatus> statusIterator = 
+        FileContext.this.listStatus(f);
       while(statusIterator.hasNext()) {
         FileStatus s = statusIterator.next();
         ContentSummary c = s.isDirectory() ? getContentSummary(s.getPath()) :
@@ -1626,24 +1627,32 @@ public final class FileContext {
      * @throws UnexpectedServerException If server implementation throws 
      *           undeclared exception to RPC server
      */
-    public Iterator<LocatedFileStatus> listFiles(
+    public RemoteIterator<LocatedFileStatus> listFiles(
         final Path f, final boolean recursive) throws AccessControlException,
         FileNotFoundException, UnsupportedFileSystemException, 
         IOException {
-      return new Iterator<LocatedFileStatus>() {
+      return new RemoteIterator<LocatedFileStatus>() {
-        private Stack<Iterator<LocatedFileStatus>> itors = 
+        private Stack<RemoteIterator<LocatedFileStatus>> itors = 
-          new Stack<Iterator<LocatedFileStatus>>();
+          new Stack<RemoteIterator<LocatedFileStatus>>();
-        Iterator<LocatedFileStatus> curItor = listLocatedStatus(f);
+        RemoteIterator<LocatedFileStatus> curItor = listLocatedStatus(f);
         LocatedFileStatus curFile;
 
         /**
-         *  {@inheritDoc}
+         * Returns <tt>true</tt> if the iterator has more files.
-         *  @return {@inheritDog} 
+         *
-         *  @throws Runtimeexception if any IOException occurs during traversal;
+         * @return <tt>true</tt> if the iterator has more files.
-         *  the IOException is set as the cause of the RuntimeException
+         * @throws AccessControlException if not allowed to access next
+         *                                file's status or locations
+         * @throws FileNotFoundException if next file does not exist any more
+         * @throws UnsupportedFileSystemException if next file's 
+         *                                        fs is unsupported
+         * @throws IOException for all other IO errors
+         *                     for example, NameNode is not avaialbe or
+         *                     NameNode throws IOException due to an error
+         *                     while getting the status or block locations
          */
         @Override
-        public boolean hasNext() {
+        public boolean hasNext() throws IOException {
           while (curFile == null) {
             if (curItor.hasNext()) {
               handleFileStat(curItor.next());
@@ -1659,44 +1668,49 @@ public final class FileContext {
         /**
          * Process the input stat.
          * If it is a file, return the file stat.
-         * If it is a directory, tranverse the directory if recursive is true;
+         * If it is a directory, traverse the directory if recursive is true;
          * ignore it if recursive is false.
          * If it is a symlink, resolve the symlink first and then process it
          * depending on if it is a file or directory.
          * @param stat input status
-         * @throws RuntimeException if any io error occurs; the io exception
+         * @throws AccessControlException if access is denied
-         * is set as the cause of RuntimeException
+         * @throws FileNotFoundException if file is not found
+         * @throws UnsupportedFileSystemException if fs is not supported
+         * @throws IOException for all other IO errors
          */
-        private void handleFileStat(LocatedFileStatus stat) {
+        private void handleFileStat(LocatedFileStatus stat)
-          try {
+        throws IOException {
-            if (stat.isFile()) { // file
+          if (stat.isFile()) { // file
-              curFile = stat;
+            curFile = stat;
-            } else if (stat.isSymlink()) { // symbolic link
+          } else if (stat.isSymlink()) { // symbolic link
-              // resolve symbolic link
+            // resolve symbolic link
-              FileStatus symstat = FileContext.this.getFileStatus(
+            FileStatus symstat = FileContext.this.getFileStatus(
-                  stat.getSymlink());
+                stat.getSymlink());
-              if (symstat.isFile() || (recursive && symstat.isDirectory())) {
+            if (symstat.isFile() || (recursive && symstat.isDirectory())) {
-                itors.push(curItor);
-                curItor = listLocatedStatus(stat.getPath());
-              }
-            } else if (recursive) { // directory
               itors.push(curItor);
               curItor = listLocatedStatus(stat.getPath());
             }
-          } catch (IOException ioe) {
+          } else if (recursive) { // directory
-            throw (RuntimeException)new RuntimeException().initCause(ioe);
+            itors.push(curItor);
+            curItor = listLocatedStatus(stat.getPath());
           }
         }
 
         /**
-         *  {@inheritDoc}
+         * Returns the next file's status with its block locations
-         *  @return {@inheritDoc} 
+         *
-         *  @throws Runtimeexception if any IOException occurs during traversal;
+         * @throws AccessControlException if not allowed to access next
-         *  the IOException is set as the cause of the RuntimeException
+         *                                file's status or locations
-         *  @exception {@inheritDoc}
+         * @throws FileNotFoundException if next file does not exist any more
+         * @throws UnsupportedFileSystemException if next file's 
+         *                                        fs is unsupported
+         * @throws IOException for all other IO errors
+         *                     for example, NameNode is not avaialbe or
+         *                     NameNode throws IOException due to an error
+         *                     while getting the status or block locations
          */
         @Override
-        public LocatedFileStatus next() {
+        public LocatedFileStatus next() throws IOException {
           if (hasNext()) {
             LocatedFileStatus result = curFile;
             curFile = null;
@@ -1704,12 +1718,6 @@ public final class FileContext {
           } 
           throw new java.util.NoSuchElementException("No more entry in " + f);
         }
-
-        @Override
-        public void remove() {
-          throw new UnsupportedOperationException("Remove is not supported");
-
-        }
       };
     }
 

src/java/org/apache/hadoop/fs/FileSystem.java

@@ -1326,18 +1326,14 @@ public abstract class FileSystem extends Configured implements Closeable {
    * If a returned status is a file, it contains the file's block locations.
    * 
    * @param f is the path
-   * @param filter path filter
    *
    * @return an iterator that traverses statuses of the files/directories 
    *         in the given path
-   * If any IO exception (for example the input directory gets deleted while
-   * listing is being executed), next() or hasNext() of the returned iterator
-   * may throw a RuntimeException with the IO exception as the cause.
    *
    * @throws FileNotFoundException If <code>f</code> does not exist
    * @throws IOException If an I/O error occurred
    */
-  public Iterator<LocatedFileStatus> listLocatedStatus(final Path f)
+  public RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f)
   throws FileNotFoundException, IOException {
     return listLocatedStatus(f, DEFAULT_FILTER);
   }
@@ -1353,50 +1349,28 @@ public abstract class FileSystem extends Configured implements Closeable {
    * @throws FileNotFoundException if <code>f</code> does not exist
    * @throws IOException if any I/O error occurred
    */
-  protected Iterator<LocatedFileStatus> listLocatedStatus(final Path f,
+  protected RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f,
       final PathFilter filter)
   throws FileNotFoundException, IOException {
-    return new Iterator<LocatedFileStatus>() {
+    return new RemoteIterator<LocatedFileStatus>() {
       private final FileStatus[] stats = listStatus(f, filter);
       private int i = 0;
 
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDog} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       */
       @Override
       public boolean hasNext() {
         return i<stats.length;
       }
 
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDoc} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       *  @exception {@inheritDoc}
-       */
       @Override
-      public LocatedFileStatus next() {
+      public LocatedFileStatus next() throws IOException {
         if (!hasNext()) {
           throw new NoSuchElementException("No more entry in " + f);
         }
         FileStatus result = stats[i++];
-        try {
+        BlockLocation[] locs = result.isFile() ?
-          BlockLocation[] locs = result.isFile() ?
             getFileBlockLocations(result.getPath(), 0, result.getLen()) :
             null;
-          return new LocatedFileStatus(result, locs);
+        return new LocatedFileStatus(result, locs);
-        } catch (IOException ioe) {
-          throw (RuntimeException)new RuntimeException().initCause(ioe);
-        }
-      }
-      
-      @Override
-      public void remove() {
-        throw new UnsupportedOperationException("Remove is not supported");
       }
     };
   }
@@ -1413,30 +1387,22 @@ public abstract class FileSystem extends Configured implements Closeable {
    * @param recursive if the subdirectories need to be traversed recursively
    *
    * @return an iterator that traverses statuses of the files
-   * If any IO exception (for example a sub-directory gets deleted while
-   * listing is being executed), next() or hasNext() of the returned iterator
-   * may throw a RuntimeException with the IO exception as the cause.
    *
    * @throws FileNotFoundException when the path does not exist;
    *         IOException see specific implementation
    */
-  public Iterator<LocatedFileStatus> listFiles(
+  public RemoteIterator<LocatedFileStatus> listFiles(
       final Path f, final boolean recursive)
   throws FileNotFoundException, IOException {
-    return new Iterator<LocatedFileStatus>() {
+    return new RemoteIterator<LocatedFileStatus>() {
-      private Stack<Iterator<LocatedFileStatus>> itors = 
+      private Stack<RemoteIterator<LocatedFileStatus>> itors = 
-        new Stack<Iterator<LocatedFileStatus>>();
+        new Stack<RemoteIterator<LocatedFileStatus>>();
-      Iterator<LocatedFileStatus> curItor = listLocatedStatus(f);
+      private RemoteIterator<LocatedFileStatus> curItor =
-      LocatedFileStatus curFile;
+        listLocatedStatus(f);
+      private LocatedFileStatus curFile;
      
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDog} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       */
       @Override
-      public boolean hasNext() {
+      public boolean hasNext() throws IOException {
         while (curFile == null) {
           if (curItor.hasNext()) {
             handleFileStat(curItor.next());
@@ -1452,34 +1418,22 @@ public abstract class FileSystem extends Configured implements Closeable {
       /**
        * Process the input stat.
        * If it is a file, return the file stat.
-       * If it is a directory, tranverse the directory if recursive is true;
+       * If it is a directory, traverse the directory if recursive is true;
        * ignore it if recursive is false.
        * @param stat input status
-       * @throws RuntimeException if any io error occurs; the io exception
+       * @throws IOException if any IO error occurs
-       * is set as the cause of RuntimeException
        */
-      private void handleFileStat(LocatedFileStatus stat) {
+      private void handleFileStat(LocatedFileStatus stat) throws IOException {
-        try {
+        if (stat.isFile()) { // file
-          if (stat.isFile()) { // file
+          curFile = stat;
-            curFile = stat;
+        } else if (recursive) { // directory
-          } else if (recursive) { // directory
+          itors.push(curItor);
-            itors.push(curItor);
+          curItor = listLocatedStatus(stat.getPath());
-            curItor = listLocatedStatus(stat.getPath());
-          }
-        } catch (IOException ioe) {
-          throw (RuntimeException)new RuntimeException().initCause(ioe);
         }
       }
 
-      /**
-       *  {@inheritDoc}
-       *  @return {@inheritDoc} 
-       *  @throws Runtimeexception if any IOException occurs during traversal;
-       *  the IOException is set as the cause of the RuntimeException
-       *  @exception {@inheritDoc}
-       */
       @Override
-      public LocatedFileStatus next() {
+      public LocatedFileStatus next() throws IOException {
         if (hasNext()) {
           LocatedFileStatus result = curFile;
           curFile = null;
@@ -1487,11 +1441,6 @@ public abstract class FileSystem extends Configured implements Closeable {
         } 
         throw new java.util.NoSuchElementException("No more entry in " + f);
       }
-
-      @Override
-      public void remove() {
-        throw new UnsupportedOperationException("Remove is not supported");
-      }
     };
   }
   

src/java/org/apache/hadoop/fs/FilterFileSystem.java

@@ -167,7 +167,7 @@ public class FilterFileSystem extends FileSystem {
   }
   
   /** List files and its block locations in a directory. */
-  public Iterator<LocatedFileStatus> listLocatedStatus(Path f)
+  public RemoteIterator<LocatedFileStatus> listLocatedStatus(Path f)
   throws IOException {
     return fs.listLocatedStatus(f);
   }

src/java/org/apache/hadoop/fs/RemoteIterator.java

@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs;
+
+import java.io.IOException;
+import java.util.NoSuchElementException;
+/**
+ * An iterator over a collection whose elements need to be fetched remotely
+ */
+public interface RemoteIterator<E> {
+  /**
+   * Returns <tt>true</tt> if the iteration has more elements.
+   *
+   * @return <tt>true</tt> if the iterator has more elements.
+   * @throws IOException if any IO error occurs
+   */
+  boolean hasNext() throws IOException;
+
+  /**
+   * Returns the next element in the iteration.
+   *
+   * @return the next element in the iteration.
+   * @throws NoSuchElementException iteration has no more elements.
+   * @throws IOException if any IO error occurs
+   */
+  E next() throws IOException;
+}

src/test/core/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java

@@ -282,7 +282,7 @@ public abstract class FileContextMainOperationsBaseTest  {
     Assert.assertEquals(0, paths.length);
     
     // test listStatus that returns an iterator
-    Iterator<FileStatus> pathsIterator = 
+    RemoteIterator<FileStatus> pathsIterator = 
       fc.listStatus(getTestRootPath(fc, "test"));
     Assert.assertEquals(getTestRootPath(fc, "test/hadoop"), 
         pathsIterator.next().getPath());

src/test/core/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java

@@ -657,7 +657,7 @@ public abstract class FileContextSymlinkBaseTest {
     // and link) and LocalFs is 3 (file, link, file crc).
     FileStatus[] stats = fc.util().listStatus(link);
     assertTrue(stats.length == 2 || stats.length == 3);
-    Iterator<FileStatus> statsItor = fc.listStatus(link);
+    RemoteIterator<FileStatus> statsItor = fc.listStatus(link);
     int dirLen = 0;
     while(statsItor.hasNext()) {
       statsItor.next();
@@ -1274,7 +1274,7 @@ public abstract class FileContextSymlinkBaseTest {
     assertTrue(fc.getFileStatus(dirViaLink).isDirectory());
     FileStatus[] stats = fc.util().listStatus(dirViaLink);
     assertEquals(0, stats.length);
-    Iterator<FileStatus> statsItor = fc.listStatus(dirViaLink);
+    RemoteIterator<FileStatus> statsItor = fc.listStatus(dirViaLink);
     assertFalse(statsItor.hasNext());
     fc.delete(dirViaLink, false);
     assertFalse(exists(fc, dirViaLink));

src/test/core/org/apache/hadoop/fs/FileContextURIBase.java

@@ -523,7 +523,8 @@ public abstract class FileContextURIBase {
     Assert.assertEquals(0, paths.length);
     
     // test listStatus that returns an iterator of FileStatus
-    Iterator<FileStatus> pathsItor = fc1.listStatus(qualifiedPath("test", fc1));
+    RemoteIterator<FileStatus> pathsItor = 
+      fc1.listStatus(qualifiedPath("test", fc1));
     Assert.assertEquals(qualifiedPath(hPrefix, fc1), pathsItor.next().getPath());
     Assert.assertFalse(pathsItor.hasNext());
 

src/test/core/org/apache/hadoop/fs/TestListFiles.java

@@ -80,7 +80,7 @@ public class TestListFiles {
     fs.mkdirs(TEST_DIR);
     writeFile(fs, FILE1, FILE_LEN);
 
-    Iterator<LocatedFileStatus> itor = fs.listFiles(
+    RemoteIterator<LocatedFileStatus> itor = fs.listFiles(
         FILE1, true);
     LocatedFileStatus stat = itor.next();
     assertFalse(itor.hasNext());
@@ -107,7 +107,7 @@ public class TestListFiles {
     fs.mkdirs(DIR1);
 
     // test empty directory
-    Iterator<LocatedFileStatus> itor = fs.listFiles(
+    RemoteIterator<LocatedFileStatus> itor = fs.listFiles(
         DIR1, true);
     assertFalse(itor.hasNext());
     itor = fs.listFiles(DIR1, false);