Suppress NoSuchFileException in LocalFileIterator

trinodb · Feb 16, 2024 · 26e0ff7 · 26e0ff7
1 parent 3521f34
commit 26e0ff7
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 27 deletions.
diff --git a/lib/trino-filesystem/src/main/java/io/trino/filesystem/local/LocalFileIterator.java b/lib/trino-filesystem/src/main/java/io/trino/filesystem/local/LocalFileIterator.java
@@ -13,49 +13,73 @@
  */
 package io.trino.filesystem.local;
 
+import com.google.common.collect.ImmutableList;
 import io.trino.filesystem.FileEntry;
 import io.trino.filesystem.FileIterator;
 import io.trino.filesystem.Location;
 
 import java.io.IOException;
+import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.util.Iterator;
 import java.util.Optional;
-import java.util.stream.Stream;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static io.trino.filesystem.local.LocalUtils.handleException;
 import static java.util.Collections.emptyIterator;
 import static java.util.Objects.requireNonNull;
 
 class LocalFileIterator
         implements FileIterator
 {
-    private final Path rootPath;
-    private final Iterator<Path> iterator;
+    private final Iterator<FileEntry> iterator;
 
     public LocalFileIterator(Location location, Path rootPath, Path path)
             throws IOException
     {
-        this.rootPath = requireNonNull(rootPath, "rootPath is null");
+        requireNonNull(rootPath, "rootPath is null");
         if (Files.isRegularFile(path)) {
             throw new IOException("Location is a file: " + location);
         }
         if (!Files.isDirectory(path)) {
             this.iterator = emptyIterator();
         }
         else {
-            try (Stream<Path> stream = Files.walk(path)) {
-                this.iterator = stream
-                        .filter(Files::isRegularFile)
-                        // materialize full list so stream can be closed
-                        .collect(toImmutableList())
-                        .iterator();
-            }
-            catch (IOException e) {
-                throw handleException(location, e);
-            }
+            ImmutableList.Builder<FileEntry> files = ImmutableList.builder();
+            Files.walkFileTree(path, new SimpleFileVisitor<>()
+            {
+                @Override
+                public FileVisitResult visitFile(Path file, BasicFileAttributes attributes)
+                        throws IOException
+                {
+                    if (Files.isRegularFile(file)) {
+                        if (!file.startsWith(rootPath)) {
+                            throw new IOException("entry is not inside of filesystem root");
+                        }
+
+                        files.add(new FileEntry(
+                                Location.of("local:///" + rootPath.relativize(file)),
+                                attributes.size(),
+                                attributes.lastModifiedTime().toInstant(),
+                                Optional.empty()));
+                    }
+                    return FileVisitResult.CONTINUE;
+                }
+
+                @Override
+                public FileVisitResult visitFileFailed(Path file, IOException e)
+                        throws IOException
+                {
+                    if (e instanceof NoSuchFileException) {
+                        // File disappeared during listing operation
+                        return FileVisitResult.CONTINUE;
+                    }
+                    throw e;
+                }
+            });
+            this.iterator = files.build().iterator();
         }
     }
 
@@ -70,15 +94,6 @@ public boolean hasNext()
     public FileEntry next()
             throws IOException
     {
-        Path path = iterator.next();
-        if (!path.startsWith(rootPath)) {
-            throw new IOException("entry is not inside of filesystem root");
-        }
-
-        return new FileEntry(
-                Location.of("local:///" + rootPath.relativize(path)),
-                Files.size(path),
-                Files.getLastModifiedTime(path).toInstant(),
-                Optional.empty());
+        return iterator.next();
     }
 }
diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java
@@ -327,7 +327,6 @@ public synchronized List<String> getAllDatabases()
             String prefix = catalogDirectory.toString();
             Set<String> databases = new HashSet<>();
 
-            // TODO this lists files recursively and may fail if e.g. table data being modified by other threads/processes
             FileIterator iterator = fileSystem.listFiles(catalogDirectory);
             while (iterator.hasNext()) {
                 Location location = iterator.next().location();