diff --git a/CHANGES.txt b/CHANGES.txt index d150d131818..43c079ba90c 100644 --- a/CHANGES.txt +++ b/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 diff --git a/src/java/org/apache/hadoop/fs/AbstractFileSystem.java b/src/java/org/apache/hadoop/fs/AbstractFileSystem.java index 9da59eed5ec..946455b843c 100644 --- a/src/java/org/apache/hadoop/fs/AbstractFileSystem.java +++ b/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 list = listStatusIterator(dst); + RemoteIterator 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 listStatusIterator(final Path f) + protected RemoteIterator listStatusIterator(final Path f) throws AccessControlException, FileNotFoundException, UnresolvedLinkException, IOException { - return new Iterator() { + return new RemoteIterator() { 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 listLocatedStatus(final Path f) + protected RemoteIterator listLocatedStatus(final Path f) throws AccessControlException, FileNotFoundException, UnresolvedLinkException, IOException { - return new Iterator() { - private Iterator itor = listStatusIterator(f); + return new RemoteIterator() { + private RemoteIterator 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()) { - locs = getFileBlockLocations( + BlockLocation[] locs = null; + if (result.isFile()) { + locs = getFileBlockLocations( result.getPath(), 0, result.getLen()); - } - 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"); + return new LocatedFileStatus(result, locs); } }; } diff --git a/src/java/org/apache/hadoop/fs/ChecksumFileSystem.java b/src/java/org/apache/hadoop/fs/ChecksumFileSystem.java index 5df36022437..c7fe2f873a2 100644 --- a/src/java/org/apache/hadoop/fs/ChecksumFileSystem.java +++ b/src/java/org/apache/hadoop/fs/ChecksumFileSystem.java @@ -504,7 +504,7 @@ public abstract class ChecksumFileSystem extends FilterFileSystem { * @throws IOException */ @Override - public Iterator listLocatedStatus(Path f) + public RemoteIterator listLocatedStatus(Path f) throws IOException { return fs.listLocatedStatus(f, DEFAULT_FILTER); } diff --git a/src/java/org/apache/hadoop/fs/FileContext.java b/src/java/org/apache/hadoop/fs/FileContext.java index 3040d7214d1..efc1d8a2d2a 100644 --- a/src/java/org/apache/hadoop/fs/FileContext.java +++ b/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 listStatus(final Path f) throws + public RemoteIterator listStatus(final Path f) throws AccessControlException, FileNotFoundException, UnsupportedFileSystemException, IOException { final Path absF = fixRelativePart(f); - return new FSLinkResolver>() { - public Iterator next(final AbstractFileSystem fs, final Path p) + return new FSLinkResolver>() { + public RemoteIterator 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 listLocatedStatus(final Path f) throws + public RemoteIterator listLocatedStatus( + final Path f) throws AccessControlException, FileNotFoundException, UnsupportedFileSystemException, IOException { final Path absF = fixRelativePart(f); - return new FSLinkResolver>() { - public Iterator next( + return new FSLinkResolver>() { + public RemoteIterator 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 statusIterator = FileContext.this.listStatus(f); + RemoteIterator 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 listFiles( + public RemoteIterator listFiles( final Path f, final boolean recursive) throws AccessControlException, FileNotFoundException, UnsupportedFileSystemException, IOException { - return new Iterator() { - private Stack> itors = - new Stack>(); - Iterator curItor = listLocatedStatus(f); + return new RemoteIterator() { + private Stack> itors = + new Stack>(); + RemoteIterator curItor = listLocatedStatus(f); LocatedFileStatus curFile; - + /** - * {@inheritDoc} - * @return {@inheritDog} - * @throws Runtimeexception if any IOException occurs during traversal; - * the IOException is set as the cause of the RuntimeException + * Returns true if the iterator has more files. + * + * @return true if the iterator has more files. + * @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 - * is set as the cause of RuntimeException + * @throws AccessControlException if access is denied + * @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) { - try { - if (stat.isFile()) { // file - curFile = stat; - } else if (stat.isSymlink()) { // symbolic link - // resolve symbolic link - FileStatus symstat = FileContext.this.getFileStatus( - stat.getSymlink()); - if (symstat.isFile() || (recursive && symstat.isDirectory())) { - itors.push(curItor); - curItor = listLocatedStatus(stat.getPath()); - } - } else if (recursive) { // directory + private void handleFileStat(LocatedFileStatus stat) + throws IOException { + if (stat.isFile()) { // file + curFile = stat; + } else if (stat.isSymlink()) { // symbolic link + // resolve symbolic link + FileStatus symstat = FileContext.this.getFileStatus( + stat.getSymlink()); + if (symstat.isFile() || (recursive && symstat.isDirectory())) { itors.push(curItor); curItor = listLocatedStatus(stat.getPath()); } - } catch (IOException ioe) { - throw (RuntimeException)new RuntimeException().initCause(ioe); + } else if (recursive) { // directory + itors.push(curItor); + curItor = listLocatedStatus(stat.getPath()); } } /** - * {@inheritDoc} - * @return {@inheritDoc} - * @throws Runtimeexception if any IOException occurs during traversal; - * the IOException is set as the cause of the RuntimeException - * @exception {@inheritDoc} + * Returns the next file's status with its block locations + * + * @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 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"); - - } }; } diff --git a/src/java/org/apache/hadoop/fs/FileSystem.java b/src/java/org/apache/hadoop/fs/FileSystem.java index 3658b04cf33..05462aeabef 100644 --- a/src/java/org/apache/hadoop/fs/FileSystem.java +++ b/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 f does not exist * @throws IOException If an I/O error occurred */ - public Iterator listLocatedStatus(final Path f) + public RemoteIterator 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 f does not exist * @throws IOException if any I/O error occurred */ - protected Iterator listLocatedStatus(final Path f, + protected RemoteIterator listLocatedStatus(final Path f, final PathFilter filter) throws FileNotFoundException, IOException { - return new Iterator() { + return new RemoteIterator() { 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 listFiles( + public RemoteIterator listFiles( final Path f, final boolean recursive) throws FileNotFoundException, IOException { - return new Iterator() { - private Stack> itors = - new Stack>(); - Iterator curItor = listLocatedStatus(f); - LocatedFileStatus curFile; + return new RemoteIterator() { + private Stack> itors = + new Stack>(); + private RemoteIterator curItor = + 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 - * is set as the cause of RuntimeException + * @throws IOException if any IO error occurs */ - private void handleFileStat(LocatedFileStatus stat) { - try { - if (stat.isFile()) { // file - curFile = stat; - } else if (recursive) { // directory - itors.push(curItor); - curItor = listLocatedStatus(stat.getPath()); - } - } catch (IOException ioe) { - throw (RuntimeException)new RuntimeException().initCause(ioe); + private void handleFileStat(LocatedFileStatus stat) throws IOException { + if (stat.isFile()) { // file + curFile = stat; + } else if (recursive) { // directory + itors.push(curItor); + curItor = listLocatedStatus(stat.getPath()); } } - /** - * {@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"); - } }; } diff --git a/src/java/org/apache/hadoop/fs/FilterFileSystem.java b/src/java/org/apache/hadoop/fs/FilterFileSystem.java index 018fa1a8476..ce47b8ba3ec 100644 --- a/src/java/org/apache/hadoop/fs/FilterFileSystem.java +++ b/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 listLocatedStatus(Path f) + public RemoteIterator listLocatedStatus(Path f) throws IOException { return fs.listLocatedStatus(f); } diff --git a/src/java/org/apache/hadoop/fs/RemoteIterator.java b/src/java/org/apache/hadoop/fs/RemoteIterator.java new file mode 100644 index 00000000000..9238c3f6fb9 --- /dev/null +++ b/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 { + /** + * Returns true if the iteration has more elements. + * + * @return true 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; +} diff --git a/src/test/core/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java b/src/test/core/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java index 2df0fbd5f71..500e50e2851 100644 --- a/src/test/core/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java +++ b/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 pathsIterator = + RemoteIterator pathsIterator = fc.listStatus(getTestRootPath(fc, "test")); Assert.assertEquals(getTestRootPath(fc, "test/hadoop"), pathsIterator.next().getPath()); diff --git a/src/test/core/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java b/src/test/core/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java index 566a88e6182..8c7885e426d 100644 --- a/src/test/core/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java +++ b/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 statsItor = fc.listStatus(link); + RemoteIterator 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 statsItor = fc.listStatus(dirViaLink); + RemoteIterator statsItor = fc.listStatus(dirViaLink); assertFalse(statsItor.hasNext()); fc.delete(dirViaLink, false); assertFalse(exists(fc, dirViaLink)); diff --git a/src/test/core/org/apache/hadoop/fs/FileContextURIBase.java b/src/test/core/org/apache/hadoop/fs/FileContextURIBase.java index 93c354e82be..5786a6653cf 100644 --- a/src/test/core/org/apache/hadoop/fs/FileContextURIBase.java +++ b/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 pathsItor = fc1.listStatus(qualifiedPath("test", fc1)); + RemoteIterator pathsItor = + fc1.listStatus(qualifiedPath("test", fc1)); Assert.assertEquals(qualifiedPath(hPrefix, fc1), pathsItor.next().getPath()); Assert.assertFalse(pathsItor.hasNext()); diff --git a/src/test/core/org/apache/hadoop/fs/TestListFiles.java b/src/test/core/org/apache/hadoop/fs/TestListFiles.java index face5d76124..d54b52b190c 100644 --- a/src/test/core/org/apache/hadoop/fs/TestListFiles.java +++ b/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 itor = fs.listFiles( + RemoteIterator 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 itor = fs.listFiles( + RemoteIterator itor = fs.listFiles( DIR1, true); assertFalse(itor.hasNext()); itor = fs.listFiles(DIR1, false);