From ad9d48e790c1354b2a95fc7ac26850aab5af272a Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Mon, 10 Aug 2026 18:07:25 +0100 Subject: [PATCH 01/10] Support parallel prefix listing in remove orphan files. --- .../iceberg/encryption/EncryptingFileIO.java | 35 ++++++ .../org/apache/iceberg/io/DelegateFileIO.java | 3 +- .../org/apache/iceberg/io/PrefixListing.java | 53 ++++++++ .../io/SupportsShallowPrefixOperations.java | 38 ++++++ .../org/apache/iceberg/aws/s3/S3FileIO.java | 49 +++++++- .../iceberg/azure/adlsv2/ADLSFileIO.java | 97 ++++++++------- .../iceberg/azure/adlsv2/ADLSLocation.java | 8 ++ .../apache/iceberg/hadoop/HadoopFileIO.java | 38 +++++- .../apache/iceberg/io/ResolvingFileIO.java | 5 + .../apache/iceberg/util/FileSystemWalker.java | 116 +++++++++++++++++- .../org/apache/iceberg/gcp/gcs/GCSFileIO.java | 39 +++++- .../actions/DeleteOrphanFilesSparkAction.java | 80 ++++++++++-- .../RemoveOrphanFilesProcedure.java | 9 +- 13 files changed, 492 insertions(+), 78 deletions(-) create mode 100644 api/src/main/java/org/apache/iceberg/io/PrefixListing.java create mode 100644 api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java diff --git a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java index 575171f85268..601cff3f1992 100644 --- a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java +++ b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java @@ -35,7 +35,9 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; @@ -52,6 +54,8 @@ public static EncryptingFileIO combine(FileIO io, EncryptionManager em) { if (io instanceof DelegateFileIO) { return new WithDelegateFileIO((DelegateFileIO) io, em); + } else if (io instanceof SupportsShallowPrefixOperations) { + return new WithSupportsShallowPrefixOperations((SupportsShallowPrefixOperations) io, em); } else if (io instanceof SupportsPrefixOperations) { return new WithSupportsPrefixOperations((SupportsPrefixOperations) io, em); } else { @@ -222,6 +226,32 @@ public EncryptionKeyMetadata copy() { } } + static class WithSupportsShallowPrefixOperations extends EncryptingFileIO + implements SupportsShallowPrefixOperations { + + private final SupportsShallowPrefixOperations prefixIo; + + WithSupportsShallowPrefixOperations(SupportsShallowPrefixOperations io, EncryptionManager em) { + super(io, em); + this.prefixIo = io; + } + + @Override + public Iterable listPrefix(String prefix) { + return prefixIo.listPrefix(prefix); + } + + @Override + public void deletePrefix(String prefix) { + prefixIo.deletePrefix(prefix); + } + + @Override + public PrefixListing listImmediate(String prefix) { + return prefixIo.listImmediate(prefix); + } + } + static class WithSupportsPrefixOperations extends EncryptingFileIO implements SupportsPrefixOperations { @@ -265,5 +295,10 @@ public Iterable listPrefix(String prefix) { public void deletePrefix(String prefix) { delegateFileIO.deletePrefix(prefix); } + + @Override + public PrefixListing listImmediate(String prefix) { + return delegateFileIO.listImmediate(prefix); + } } } diff --git a/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java b/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java index 4b97b3cb2874..3199b2952946 100644 --- a/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java +++ b/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java @@ -22,4 +22,5 @@ * This interface is intended as an extension for FileIO implementations that support being a * delegate target. */ -public interface DelegateFileIO extends FileIO, SupportsPrefixOperations, SupportsBulkOperations {} +public interface DelegateFileIO + extends FileIO, SupportsShallowPrefixOperations, SupportsBulkOperations {} diff --git a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java new file mode 100644 index 000000000000..d08ab36bec99 --- /dev/null +++ b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java @@ -0,0 +1,53 @@ +/* + * 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.iceberg.io; + +/** + * Result of a shallow (single-level) prefix listing, separating files found immediately under the + * prefix from sub-prefixes that would be expanded further. + * + *

For key/value object stores this maps directly to a delimited listing where {@link #files()} + * corresponds to entries returned as objects and {@link #subPrefixes()} corresponds to entries + * returned as common prefixes. For hierarchical file systems, {@link #files()} contains regular + * files found directly under the prefix and {@link #subPrefixes()} contains sub-directory + * locations. + */ +public interface PrefixListing { + + /** Files found immediately under the listed prefix. */ + Iterable files(); + + /** Sub-prefixes one level below the listed prefix. Each is a location suitable for re-listing. */ + Iterable subPrefixes(); + + /** Create a {@link PrefixListing} from the given iterables. */ + static PrefixListing of(Iterable files, Iterable subPrefixes) { + return new PrefixListing() { + @Override + public Iterable files() { + return files; + } + + @Override + public Iterable subPrefixes() { + return subPrefixes; + } + }; + } +} diff --git a/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java b/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java new file mode 100644 index 000000000000..301803117523 --- /dev/null +++ b/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java @@ -0,0 +1,38 @@ +/* + * 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.iceberg.io; + +/** + * Extension of {@link SupportsPrefixOperations} for FileIO implementations that can list a single + * directory level natively (e.g. delimited listing on object stores). + */ +public interface SupportsShallowPrefixOperations extends SupportsPrefixOperations { + + /** + * List the immediate children of a prefix — one directory level down. + * + *

For hierarchical file systems this returns direct file and sub-directory children of the + * given directory. For key/value object stores this maps to a delimited listing where + * sub-prefixes correspond to distinct path segments appearing immediately after the prefix. + * + * @param prefix prefix to list; hierarchical stores may require this to match a directory + * @return files and sub-prefixes found immediately under the given prefix + */ + PrefixListing listImmediate(String prefix); +} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java index 50e507b0cd8b..633c43b82344 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java @@ -44,6 +44,7 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsRecoveryOperations; import org.apache.iceberg.io.SupportsStorageCredentials; @@ -76,10 +77,12 @@ import software.amazon.awssdk.services.s3.model.GetObjectTaggingRequest; import software.amazon.awssdk.services.s3.model.GetObjectTaggingResponse; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; import software.amazon.awssdk.services.s3.model.ObjectIdentifier; import software.amazon.awssdk.services.s3.model.ObjectVersion; import software.amazon.awssdk.services.s3.model.PutObjectTaggingRequest; import software.amazon.awssdk.services.s3.model.S3Exception; +import software.amazon.awssdk.services.s3.model.S3Object; import software.amazon.awssdk.services.s3.model.Tag; import software.amazon.awssdk.services.s3.model.Tagging; import software.amazon.awssdk.services.s3.paginators.ListObjectVersionsIterable; @@ -343,15 +346,49 @@ public Iterable listPrefix(String prefix) { return () -> client.s3().listObjectsV2Paginator(request).stream() .flatMap(r -> r.contents().stream()) - .map( - o -> - new FileInfo( - String.format("%s://%s/%s", s3uri.scheme(), s3uri.bucket(), o.key()), - o.size(), - o.lastModified().toEpochMilli())) + .map(o -> createFileInfo(s3uri, o)) .iterator(); } + @Override + public PrefixListing listImmediate(String prefix) { + PrefixedS3Client client = clientForStoragePath(prefix); + + S3URI uri = new S3URI(prefix, client.s3FileIOProperties().bucketToAccessPointMapping()); + if (uri.useS3DirectoryBucket() + && client.s3FileIOProperties().isS3DirectoryBucketListPrefixAsDirectory()) { + uri = uri.toDirectoryPath(); + } + + S3URI s3uri = uri; + ListObjectsV2Request request = + ListObjectsV2Request.builder() + .bucket(s3uri.bucket()) + .prefix(s3uri.key()) + .delimiter("/") + .build(); + + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + for (ListObjectsV2Response response : client.s3().listObjectsV2Paginator(request)) { + response.contents().forEach(o -> files.add(createFileInfo(s3uri, o))); + response + .commonPrefixes() + .forEach(commonPrefix -> subPrefixes.add(toUri(s3uri, commonPrefix.prefix()))); + } + + return PrefixListing.of(files, subPrefixes); + } + + private FileInfo createFileInfo(S3URI s3uri, S3Object object) { + return new FileInfo( + toUri(s3uri, object.key()), object.size(), object.lastModified().toEpochMilli()); + } + + private String toUri(S3URI s3uri, String key) { + return String.format("%s://%s/%s", s3uri.scheme(), s3uri.bucket(), key); + } + /** * This method provides a "best-effort" to delete all objects under the given prefix. * diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java index c6627f21c783..f375ad836b0a 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java @@ -25,7 +25,9 @@ import com.azure.storage.file.datalake.DataLakeFileSystemClientBuilder; import com.azure.storage.file.datalake.models.DataLakeStorageException; import com.azure.storage.file.datalake.models.ListPathsOptions; +import com.azure.storage.file.datalake.models.PathItem; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; @@ -36,10 +38,11 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.metrics.MetricsContext; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; -import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.apache.iceberg.util.SerializableFunction; +import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.util.SerializableMap; import org.apache.iceberg.util.Tasks; import org.apache.iceberg.util.ThreadPools; @@ -47,7 +50,7 @@ import org.slf4j.LoggerFactory; /** FileIO implementation backed by Azure Data Lake Storage Gen2. */ -public class ADLSFileIO implements DelegateFileIO { +public class ADLSFileIO implements DelegateFileIO, SupportsShallowPrefixOperations { private static final Logger LOG = LoggerFactory.getLogger(ADLSFileIO.class); private static final String DEFAULT_METRICS_IMPL = @@ -59,8 +62,6 @@ public class ADLSFileIO implements DelegateFileIO { private MetricsContext metrics = MetricsContext.nullMetrics(); private SerializableMap properties; private VendedAdlsCredentialProvider vendedAdlsCredentialProvider; - private SerializableFunction clientSupplier; - private transient volatile Map clientCache; /** * No-arg constructor to load the FileIO dynamically. @@ -74,23 +75,6 @@ public ADLSFileIO() {} this.azureProperties = azureProperties; } - /** - * Constructor with custom DataLakeFileSystemClient function. - * - *

Unlike the no-arg constructor, this constructor initializes properties and azureProperties - * immediately, allowing immediate use without calling {@link ADLSFileIO#initialize(Map)}. - * - *

The function receives an {@link ADLSLocation} and should return an appropriate {@link - * DataLakeFileSystemClient} for that location. Clients are cached per storage account and - * container combination. - * - * @param clientSupplier function that creates a client for a given location - */ - public ADLSFileIO(SerializableFunction clientSupplier) { - this.clientSupplier = clientSupplier; - initialize(Maps.newHashMap()); - } - @Override public InputFile newInputFile(String path) { return new ADLSInputFile(path, fileClient(path), azureProperties, metrics); @@ -130,22 +114,6 @@ public DataLakeFileSystemClient client(String path) { @VisibleForTesting DataLakeFileSystemClient client(ADLSLocation location) { - if (clientCache == null) { - synchronized (this) { - if (clientCache == null) { - clientCache = Maps.newConcurrentMap(); - } - } - } - String cacheKey = location.host() + "/" + location.container().orElse(""); - return clientCache.computeIfAbsent(cacheKey, k -> buildClient(location)); - } - - private DataLakeFileSystemClient buildClient(ADLSLocation location) { - if (clientSupplier != null) { - return clientSupplier.apply(location); - } - DataLakeFileSystemClientBuilder clientBuilder = new DataLakeFileSystemClientBuilder().httpClient(HTTP); @@ -215,6 +183,7 @@ public void deleteFiles(Iterable pathsToDelete) throws BulkDeletionFailu @Override public Iterable listPrefix(String prefix) { ADLSLocation location = new ADLSLocation(prefix); + String baseUri = toBaseUri(location); ListPathsOptions options = new ListPathsOptions(); options.setPath(location.path()); @@ -224,12 +193,7 @@ public Iterable listPrefix(String prefix) { try { return client(location).listPaths(options, null).stream() .filter(pathItem -> !pathItem.isDirectory()) - .map( - pathItem -> - new FileInfo( - pathItem.getName(), - pathItem.getContentLength(), - pathItem.getCreationTime().toInstant().toEpochMilli())) + .map(pathItem -> createFileInfo(baseUri, pathItem)) .iterator(); } catch (DataLakeStorageException e) { // other FileIO implementations return an empty iterator if nothing @@ -242,6 +206,51 @@ public Iterable listPrefix(String prefix) { }; } + @Override + public PrefixListing listImmediate(String prefix) { + ADLSLocation location = new ADLSLocation(prefix); + String baseUri = toBaseUri(location); + + ListPathsOptions options = new ListPathsOptions(); + options.setPath(location.path()); + options.setRecursive(false); + + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + + try { + client(location) + .listPaths(options, null) + .forEach( + pathItem -> { + if (pathItem.isDirectory()) { + subPrefixes.add(baseUri + pathItem.getName()); + } else { + files.add(createFileInfo(baseUri, pathItem)); + } + }); + } catch (DataLakeStorageException e) { + if (e.getStatusCode() != 404) { + throw e; + } + return PrefixListing.of(Collections.emptyList(), Collections.emptyList()); + } + + return PrefixListing.of(files, subPrefixes); + } + + private String toBaseUri(ADLSLocation location) { + return String.format( + "%s://%s@%s/", location.scheme(), location.container().orElse(""), location.host()); + } + + private FileInfo createFileInfo(String baseUri, PathItem pathItem) { + return new FileInfo( + baseUri + pathItem.getName(), + pathItem.getContentLength(), + pathItem.getCreationTime().toInstant().toEpochMilli()); + } + @Override public void deletePrefix(String prefix) { ADLSLocation location = new ADLSLocation(prefix); diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java index 6fe2629774dc..cfb8a3998f59 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java @@ -46,6 +46,7 @@ class ADLSLocation { private static final Pattern URI_PATTERN = Pattern.compile("^(abfss?|wasbs?)://([^/?#]+)(.*)?$"); + private final String scheme; private final String storageAccount; private final String container; private final String path; @@ -63,6 +64,8 @@ class ADLSLocation { ValidationException.check(matcher.matches(), "Invalid ADLS URI: %s", location); + this.scheme = matcher.group(1); + String authority = matcher.group(2); String[] parts = authority.split("@", -1); if (parts.length > 1) { @@ -79,6 +82,11 @@ class ADLSLocation { this.path = uriPath == null ? "" : uriPath.startsWith("/") ? uriPath.substring(1) : uriPath; } + /** Returns the scheme used for accessing this location. */ + public String scheme() { + return scheme; + } + /** Returns Azure storage account. */ public String storageAccount() { return storageAccount; diff --git a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java index 877290f48e3f..6d94a34c209b 100644 --- a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java +++ b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java @@ -22,11 +22,13 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.RemoteIterator; @@ -36,7 +38,9 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Streams; import org.apache.iceberg.util.SerializableMap; import org.apache.iceberg.util.SerializableSupplier; @@ -157,12 +161,7 @@ public Iterable listPrefix(String prefix) { try { return Streams.stream( new AdaptingIterator<>(fs.listFiles(prefixToList, true /* recursive */))) - .map( - fileStatus -> - new FileInfo( - fileStatus.getPath().toString(), - fileStatus.getLen(), - fileStatus.getModificationTime())) + .map(this::createFileInfo) .iterator(); } catch (IOException e) { throw new UncheckedIOException(e); @@ -170,6 +169,33 @@ public Iterable listPrefix(String prefix) { }; } + @Override + public PrefixListing listImmediate(String prefix) { + Path prefixToList = new Path(prefix); + FileSystem fs = Util.getFs(prefixToList, getConf()); + + try { + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + + for (FileStatus status : fs.listStatus(prefixToList)) { + if (status.isDirectory()) { + subPrefixes.add(status.getPath().toString()); + } else { + files.add(createFileInfo(status)); + } + } + + return PrefixListing.of(files, subPrefixes); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private FileInfo createFileInfo(FileStatus status) { + return new FileInfo(status.getPath().toString(), status.getLen(), status.getModificationTime()); + } + @Override public void deletePrefix(String prefix) { Path prefixToDelete = new Path(prefix); diff --git a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java index c27c609d94f8..7304f2d479c3 100644 --- a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java +++ b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java @@ -279,6 +279,11 @@ public void deletePrefix(String prefix) { io(prefix).deletePrefix(prefix); } + @Override + public PrefixListing listImmediate(String prefix) { + return io(prefix).listImmediate(prefix); + } + @Override public void setCredentials(List credentials) { Preconditions.checkArgument(credentials != null, "Invalid storage credentials: null"); diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index 479c8e221e12..c2a063f43877 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.Serializable; import java.io.UncheckedIOException; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -35,7 +36,10 @@ import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.hadoop.HiddenPathFilter; import org.apache.iceberg.io.FileInfo; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; +import org.apache.iceberg.relocated.com.google.common.collect.Iterators; import org.apache.iceberg.relocated.com.google.common.collect.Lists; /** @@ -63,19 +67,123 @@ public static void listDirRecursivelyWithFileIO( Map specs, Predicate filter, Consumer fileConsumer) { + listDirRecursivelyWithFileIO(io, dir, specs, filter).forEachRemaining(fileConsumer); + } + + public static Iterator listDirRecursivelyWithFileIO( + SupportsPrefixOperations io, + String dir, + Map specs, + Predicate filter) { PathFilter pathFilter = PartitionAwareHiddenPathFilter.forSpecs(specs); String listPath = dir; if (!dir.endsWith("/")) { listPath = dir + "/"; } - Iterable files = io.listPrefix(listPath); - for (FileInfo file : files) { - Path path = new Path(file.location()); - if (!isHiddenPath(dir, path, pathFilter) && filter.test(file)) { + Iterator files = io.listPrefix(listPath).iterator(); + return Iterators.transform( + Iterators.filter( + files, + file -> { + Path path = new Path(file.location()); + return !isHiddenPath(dir, path, pathFilter) && filter.test(file); + }), + FileInfo::location); + } + + /** + * Recursively lists files in the specified directory that satisfy the given conditions. Use + * {@link PartitionAwareHiddenPathFilter} to filter out hidden paths. + * + *

Provides the same depth-and-fan-out controls as {@link #listDirRecursivelyWithHadoop}: + * + *

    + *
  • Stops traversal when the maximum recursion depth is reached and adds the current location + * to the pending list via {@code directoryConsumer}. + *
  • Stops traversal when the number of direct sub-prefixes at a level exceeds the threshold + * and adds those sub-prefixes to the pending list. + *
+ * + * @param io FileIO implementation that supports shallow prefix listing + * @param dir the starting prefix to traverse + * @param specs partition specs used to preserve partition-name-based hidden paths + * @param filter file filter; only files satisfying this condition will be collected + * @param maxDepth maximum recursion depth + * @param maxDirectSubDirs upper limit of sub-prefixes that can be processed directly + * @param directoryConsumer consumer for sub-prefixes that were not expanded further + * @param fileConsumer consumer for qualifying file locations + */ + public static void listDirRecursivelyWithFileIO( + SupportsShallowPrefixOperations io, + String dir, + Map specs, + Predicate filter, + int maxDepth, + int maxDirectSubDirs, + Consumer directoryConsumer, + Consumer fileConsumer) { + PathFilter pathFilter = PartitionAwareHiddenPathFilter.forSpecs(specs); + listDirRecursivelyWithFileIO( + io, + dir, + dir, + pathFilter, + filter, + maxDepth, + maxDirectSubDirs, + directoryConsumer, + fileConsumer); + } + + private static void listDirRecursivelyWithFileIO( + SupportsShallowPrefixOperations io, + String baseDir, + String dir, + PathFilter pathFilter, + Predicate filter, + int maxDepth, + int maxDirectSubDirs, + Consumer directoryConsumer, + Consumer fileConsumer) { + if (maxDepth <= 0) { + directoryConsumer.accept(dir); + return; + } + + PrefixListing listing = io.listImmediate(dir); + + List subDirs = Lists.newArrayList(); + for (String subPrefix : listing.subPrefixes()) { + if (!isHiddenPath(baseDir, new Path(subPrefix), pathFilter)) { + subDirs.add(subPrefix); + } + } + + for (FileInfo file : listing.files()) { + Path filePath = new Path(file.location()); + if (!isHiddenPath(baseDir, filePath, pathFilter) && filter.test(file)) { fileConsumer.accept(file.location()); } } + + if (subDirs.size() > maxDirectSubDirs) { + subDirs.forEach(directoryConsumer); + return; + } + + for (String subDir : subDirs) { + listDirRecursivelyWithFileIO( + io, + baseDir, + subDir, + pathFilter, + filter, + maxDepth - 1, + maxDirectSubDirs, + directoryConsumer, + fileConsumer); + } } /** diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java index 5e040943f703..e11b16ca9a5a 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java @@ -43,6 +43,7 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsStorageCredentials; import org.apache.iceberg.metrics.MetricsContext; @@ -297,15 +298,41 @@ public Iterable listPrefix(String prefix) { .storage() .list(location.bucket(), Storage.BlobListOption.prefix(location.prefix())) .streamAll() - .map( - blob -> - new FileInfo( - String.format("gs://%s/%s", blob.getBucket(), blob.getName()), - blob.getSize(), - createTimeMillis(blob))) + .map(this::createFileInfo) .iterator(); } + @Override + public PrefixListing listImmediate(String prefix) { + GCSLocation location = new GCSLocation(prefix); + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + clientForStoragePath(prefix) + .storage() + .list( + location.bucket(), + Storage.BlobListOption.prefix(location.prefix()), + Storage.BlobListOption.currentDirectory()) + .streamAll() + .forEach( + blob -> { + if (blob.isDirectory()) { + subPrefixes.add(createUri(blob)); + } else { + files.add(createFileInfo(blob)); + } + }); + return PrefixListing.of(files, subPrefixes); + } + + private FileInfo createFileInfo(Blob blob) { + return new FileInfo(createUri(blob), blob.getSize(), createTimeMillis(blob)); + } + + private String createUri(Blob blob) { + return String.format("gs://%s/%s", blob.getBucket(), blob.getName()); + } + private long createTimeMillis(Blob blob) { if (blob.getCreateTimeOffsetDateTime() == null) { return 0; diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index b47922820d21..2d154a5d965c 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,6 +44,7 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -131,6 +132,7 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; + private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -222,6 +224,15 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } + public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { + Preconditions.checkArgument( + newMaxSeedDepth >= 0, + "Prefix listing max seed depth must be non-negative: %s", + newMaxSeedDepth); + this.prefixListingMaxSeedDepth = newMaxSeedDepth; + return this; + } + private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -422,17 +433,41 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - matchingFiles::add); + List seedPrefixes = Lists.newArrayList(); + if (prefixListingMaxSeedDepth == 0) { + seedPrefixes.add(location); + } else { + Preconditions.checkArgument( + table.io() instanceof SupportsShallowPrefixOperations, + "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" + + " shallow prefix operations.", + table.io()); + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsShallowPrefixOperations) table.io(), + location, + table.specs(), + predicate, + prefixListingMaxSeedDepth, + MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, + seedPrefixes::add, + matchingFiles::add); + } - JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); + int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); + JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); + ListPrefixes listPrefixes = + new ListPrefixes( + (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); + JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); + + if (matchingFiles.isEmpty()) { + return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); + } + + JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -523,6 +558,31 @@ public Iterator call(Iterator dirs) throws Exception { } } + private static class ListPrefixes implements FlatMapFunction, String> { + + private final SupportsPrefixOperations io; + private final long olderThanTimestamp; + private final Map specs; + + ListPrefixes( + SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { + this.io = io; + this.olderThanTimestamp = olderThanTimestamp; + this.specs = specs; + } + + @Override + public Iterator call(Iterator prefixes) { + Predicate predicate = + file -> file.createdAtMillis() < olderThanTimestamp; + return Iterators.concat( + Iterators.transform( + prefixes, + prefix -> + FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); + } + } + private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index 89e218c10360..e2622f5c9647 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,6 +80,10 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); + // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // listing. Only applies when prefix_listing is true. Default is 0 (single seed). + private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = + optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -93,7 +97,8 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM + STREAM_RESULTS_PARAM, + PREFIX_LISTING_MAX_SEED_DEPTH_PARAM }; private static final StructType OUTPUT_TYPE = @@ -149,6 +154,7 @@ public Iterator call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); + int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -197,6 +203,7 @@ public Iterator call(InternalRow args) { } action.usePrefixListing(prefixListing); + action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); From a1e3923e0a3e392507bf0f390b4bff5d72c7d5e0 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Tue, 11 Aug 2026 10:16:37 +0100 Subject: [PATCH 02/10] tests --- .../TestRemoveOrphanFilesProcedure.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index a5ac8a7e01ac..13764e813350 100644 --- a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -667,6 +667,90 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } + @TestTemplate + public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { + if (catalogName.equals("testhadoop")) { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + } else { + sql( + "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", + tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); + } + + sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); + sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); + + Table table = validationCatalog.loadTable(tableIdent); + String dataLocation = table.location() + "/data"; + + // produce orphan files in the data location using parquet + sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); + sql("INSERT INTO TABLE p VALUES (1)"); + sql("INSERT INTO TABLE p VALUES (2)"); + + // wait to ensure files are old enough + waitUntilAfter(System.currentTimeMillis()); + + Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); + + List output = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output).as("Should find orphan files in the data folder").hasSize(2); + + // subsequent call should find nothing + List output2 = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output2).as("Should be no more orphan files").isEmpty(); + + // seed depth of 0 (default single-seed behavior) should also work + sql("INSERT INTO TABLE p VALUES (3)"); + waitUntilAfter(System.currentTimeMillis()); + Timestamp laterTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); + + List output3 = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, laterTimestamp, 0); + assertThat(output3).as("Should find the new orphan file with single-seed listing").hasSize(1); + + assertEquals( + "Should have expected rows", + ImmutableList.of(row(1L, "a"), row(2L, "b")), + sql("SELECT * FROM %s ORDER BY id", tableName)); + } + + @TestTemplate + public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + + assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, -1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Prefix listing max seed depth must be non-negative"); + } + @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { From 97a1ab091e3cbacf9dddd7c8129937055d48ac16 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Tue, 18 Aug 2026 09:23:37 +0100 Subject: [PATCH 03/10] changes --- .../iceberg/azure/adlsv2/TestADLSFileIO.java | 3 +- .../iceberg/azure/adlsv2/ADLSFileIO.java | 37 +++++ .../apache/iceberg/util/FileSystemWalker.java | 3 +- .../iceberg/hadoop/TestHadoopFileIO.java | 24 ++++ .../iceberg/util/TestFileSystemWalker.java | 130 ++++++++++++++++++ .../iceberg/gcp/auth/GoogleAuthSession.java | 1 + .../TestRemoveOrphanFilesProcedure.java | 66 +++++++++ .../actions/DeleteOrphanFilesSparkAction.java | 80 +++++++++-- .../RemoveOrphanFilesProcedure.java | 9 +- .../TestRemoveOrphanFilesProcedure.java | 66 +++++++++ .../actions/DeleteOrphanFilesSparkAction.java | 80 +++++++++-- .../RemoveOrphanFilesProcedure.java | 9 +- .../actions/TestRemoveOrphanFilesAction.java | 41 ++++++ 13 files changed, 525 insertions(+), 24 deletions(-) diff --git a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java index 621813a25574..45759a1eaebe 100644 --- a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java +++ b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java @@ -169,7 +169,8 @@ public void testListPrefixOperations() { // assert that only files were returned and not directories FileInfo fileInfo = result.next(); - assertThat(fileInfo.location()).isEqualTo("dir/file"); + assertThat(fileInfo.location()) + .isEqualTo("abfs://container@account.dfs.core.windows.net/dir/file"); assertThat(fileInfo.size()).isEqualTo(123L); assertThat(fileInfo.createdAtMillis()).isEqualTo(now.toInstant().toEpochMilli()); diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java index f375ad836b0a..4b0af46a7433 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java @@ -43,6 +43,8 @@ import org.apache.iceberg.metrics.MetricsContext; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.util.SerializableFunction; import org.apache.iceberg.util.SerializableMap; import org.apache.iceberg.util.Tasks; import org.apache.iceberg.util.ThreadPools; @@ -62,6 +64,8 @@ public class ADLSFileIO implements DelegateFileIO, SupportsShallowPrefixOperatio private MetricsContext metrics = MetricsContext.nullMetrics(); private SerializableMap properties; private VendedAdlsCredentialProvider vendedAdlsCredentialProvider; + private SerializableFunction clientSupplier; + private transient volatile Map clientCache; /** * No-arg constructor to load the FileIO dynamically. @@ -75,6 +79,23 @@ public ADLSFileIO() {} this.azureProperties = azureProperties; } + /** + * Constructor with custom DataLakeFileSystemClient function. + * + *

Unlike the no-arg constructor, this constructor initializes properties and azureProperties + * immediately, allowing immediate use without calling {@link ADLSFileIO#initialize(Map)}. + * + *

The function receives an {@link ADLSLocation} and should return an appropriate {@link + * DataLakeFileSystemClient} for that location. Clients are cached per storage account and + * container combination. + * + * @param clientSupplier function that creates a client for a given location + */ + public ADLSFileIO(SerializableFunction clientSupplier) { + this.clientSupplier = clientSupplier; + initialize(Maps.newHashMap()); + } + @Override public InputFile newInputFile(String path) { return new ADLSInputFile(path, fileClient(path), azureProperties, metrics); @@ -114,6 +135,22 @@ public DataLakeFileSystemClient client(String path) { @VisibleForTesting DataLakeFileSystemClient client(ADLSLocation location) { + if (clientCache == null) { + synchronized (this) { + if (clientCache == null) { + clientCache = Maps.newConcurrentMap(); + } + } + } + String cacheKey = location.host() + "/" + location.container().orElse(""); + return clientCache.computeIfAbsent(cacheKey, k -> buildClient(location)); + } + + private DataLakeFileSystemClient buildClient(ADLSLocation location) { + if (clientSupplier != null) { + return clientSupplier.apply(location); + } + DataLakeFileSystemClientBuilder clientBuilder = new DataLakeFileSystemClientBuilder().httpClient(HTTP); diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index c2a063f43877..8beb7a308637 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -151,7 +151,8 @@ private static void listDirRecursivelyWithFileIO( return; } - PrefixListing listing = io.listImmediate(dir); + String listPath = dir.endsWith("/") ? dir : dir + "/"; + PrefixListing listing = io.listImmediate(listPath); List subDirs = Lists.newArrayList(); for (String subPrefix : listing.subPrefixes()) { diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java index 554ed625b9e3..f089ceb86b07 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java @@ -38,6 +38,7 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.FileIOParser; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -86,6 +87,29 @@ public void testListPrefix() { .isEqualTo(totalFiles); } + @Test + public void testListImmediate() throws IOException { + Path parent = new Path(tempDir.toURI()); + + Path subDir = new Path(parent, "sub"); + fs.mkdirs(subDir); + fs.createNewFile(new Path(subDir, "nested.txt")); + + Path immediateFile1 = new Path(parent, "a.txt"); + Path immediateFile2 = new Path(parent, "b.txt"); + fs.createNewFile(immediateFile1); + fs.createNewFile(immediateFile2); + + PrefixListing listing = hadoopFileIO.listImmediate(parent.toUri().toString()); + + assertThat(listing.files()) + .extracting(fileInfo -> new Path(fileInfo.location()).getName()) + .containsExactlyInAnyOrder("a.txt", "b.txt"); + assertThat(listing.subPrefixes()) + .extracting(prefix -> new Path(prefix).getName()) + .containsExactly("sub"); + } + @Test public void testFileExists() throws IOException { Path parent = new Path(tempDir.toURI()); diff --git a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java index ba03c073c65a..a66ade3eeb21 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java +++ b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java @@ -36,7 +36,9 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -248,6 +250,99 @@ private void listFilesRecursively( assertThat(remainingDirs).isEmpty(); } + @Test + public void testShallowListDirRecursivelyWithFileIO() { + List foundFiles = Lists.newArrayList(); + List remainingDirs = Lists.newArrayList(); + Predicate fileFilter = fileInfo -> fileInfo.location().endsWith(".txt"); + FileSystemWalker.listDirRecursivelyWithFileIO( + fileIO, + basePath, + specs, + fileFilter, + Integer.MAX_VALUE, + Integer.MAX_VALUE, + remainingDirs::add, + foundFiles::add); + + assertThat(foundFiles).hasSize(4); + assertThat(foundFiles).contains(Paths.get("file://", basePath, "file1.txt").toString()); + assertThat(foundFiles) + .contains(Paths.get("file://", basePath, "normal_dir/file2.txt").toString()); + assertThat(foundFiles) + .contains(Paths.get("file://", basePath, "normal_dir/dep1/file3.txt").toString()); + assertThat(foundFiles) + .contains(Paths.get("file://", basePath, "normal_dir_1/file4.txt").toString()); + assertThat(remainingDirs).isEmpty(); + } + + @Test + public void testShallowListDirRecursivelyWithFileIOMaxDepth() { + List foundFiles = Lists.newArrayList(); + List remainingDirs = Lists.newArrayList(); + Predicate fileFilter = fileInfo -> fileInfo.location().endsWith(".txt"); + FileSystemWalker.listDirRecursivelyWithFileIO( + fileIO, + basePath, + specs, + fileFilter, + 2, // maxDepth + 10, // maxDirectSubDirs + remainingDirs::add, + foundFiles::add); + + assertThat(foundFiles).hasSize(3); + assertThat(foundFiles).contains(Paths.get("file://", basePath, "file1.txt").toString()); + assertThat(foundFiles) + .contains(Paths.get("file://", basePath, "normal_dir/file2.txt").toString()); + assertThat(foundFiles) + .contains(Paths.get("file://", basePath, "normal_dir_1/file4.txt").toString()); + assertThat(remainingDirs).hasSize(1); + assertThat(remainingDirs) + .contains(Paths.get("file://", basePath, "normal_dir/dep1").toString()); + } + + @Test + public void testShallowListDirRecursivelyWithFileIOMaxDirectSubDirs() { + List foundFiles = Lists.newArrayList(); + List remainingDirs = Lists.newArrayList(); + Predicate fileFilter = fileInfo -> fileInfo.location().endsWith(".txt"); + FileSystemWalker.listDirRecursivelyWithFileIO( + fileIO, + basePath, + specs, + fileFilter, + 2, // maxDepth + 1, // maxDirectSubDirs + remainingDirs::add, + foundFiles::add); + + assertThat(foundFiles).hasSize(1); + assertThat(foundFiles).contains(Paths.get("file://", basePath, "file1.txt").toString()); + assertThat(remainingDirs).hasSize(3); + assertThat(remainingDirs).contains(Paths.get("file://", basePath, "normal_dir").toString()); + assertThat(remainingDirs).contains(Paths.get("file://", basePath, "normal_dir_1").toString()); + assertThat(remainingDirs).contains(Paths.get("file://", basePath, "hidden_dir").toString()); + } + + @Test + public void testShallowListDirRecursivelyWithFileIONormalizesTrailingSlash() { + RecordingShallowFileIO recordingIO = new RecordingShallowFileIO(); + FileSystemWalker.listDirRecursivelyWithFileIO( + recordingIO, + "s3://bucket/table", + null, + fileInfo -> true, + 1, + Integer.MAX_VALUE, + dir -> {}, + file -> {}); + + // walker must normalize the seed prefix so object-store list calls do not accidentally match + // sibling prefixes like "s3://bucket/table_backup/" + assertThat(recordingIO.calls).containsExactly("s3://bucket/table/"); + } + @Test public void testListDirRecursivelyWithFileIOBucketRootBaseDir() { assertThat( @@ -309,6 +404,41 @@ private static List listWithMockFileIO(String baseDir, FileInfo... entri return foundFiles; } + private static class RecordingShallowFileIO implements SupportsShallowPrefixOperations { + private final List calls = Lists.newArrayList(); + + @Override + public PrefixListing listImmediate(String prefix) { + calls.add(prefix); + return PrefixListing.of(ImmutableList.of(), ImmutableList.of()); + } + + @Override + public Iterable listPrefix(String prefix) { + throw new UnsupportedOperationException(); + } + + @Override + public void deletePrefix(String prefix) { + throw new UnsupportedOperationException(); + } + + @Override + public InputFile newInputFile(String path) { + throw new UnsupportedOperationException(); + } + + @Override + public OutputFile newOutputFile(String path) { + throw new UnsupportedOperationException(); + } + + @Override + public void deleteFile(String path) { + throw new UnsupportedOperationException(); + } + } + private static class StaticPrefixFileIO implements SupportsPrefixOperations { private final Iterable entries; diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java index c81053613da4..14e2b7f0730e 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java @@ -70,6 +70,7 @@ public HTTPRequest authenticate(HTTPRequest request) { HTTPHeaders.of( HTTPHeaders.HTTPHeader.of( "Authorization", "Bearer " + token.getTokenValue()))); + return newHeaders.equals(request.headers()) ? request : ImmutableHTTPRequest.builder().from(request).headers(newHeaders).build(); diff --git a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index c17dbad71100..a6d4b11a28a2 100644 --- a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -668,6 +668,72 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } + @TestTemplate + public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { + if (catalogName.equals("testhadoop")) { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + } else { + sql( + "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", + tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); + } + + sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); + sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); + + Table table = validationCatalog.loadTable(tableIdent); + String dataLocation = table.location() + "/data"; + + sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); + sql("INSERT INTO TABLE p VALUES (1)"); + sql("INSERT INTO TABLE p VALUES (2)"); + + waitUntilAfter(System.currentTimeMillis()); + + Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); + + List output = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output).as("Should find orphan files in the data folder").hasSize(2); + + List output2 = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output2).as("Should be no more orphan files").isEmpty(); + + assertEquals( + "Should have expected rows", + ImmutableList.of(row(1L, "a"), row(2L, "b")), + sql("SELECT * FROM %s ORDER BY id", tableName)); + } + + @TestTemplate + public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + + assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, -1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Prefix listing max seed depth must be non-negative"); + } + @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index b47922820d21..2d154a5d965c 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,6 +44,7 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -131,6 +132,7 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; + private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -222,6 +224,15 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } + public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { + Preconditions.checkArgument( + newMaxSeedDepth >= 0, + "Prefix listing max seed depth must be non-negative: %s", + newMaxSeedDepth); + this.prefixListingMaxSeedDepth = newMaxSeedDepth; + return this; + } + private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -422,17 +433,41 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - matchingFiles::add); + List seedPrefixes = Lists.newArrayList(); + if (prefixListingMaxSeedDepth == 0) { + seedPrefixes.add(location); + } else { + Preconditions.checkArgument( + table.io() instanceof SupportsShallowPrefixOperations, + "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" + + " shallow prefix operations.", + table.io()); + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsShallowPrefixOperations) table.io(), + location, + table.specs(), + predicate, + prefixListingMaxSeedDepth, + MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, + seedPrefixes::add, + matchingFiles::add); + } - JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); + int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); + JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); + ListPrefixes listPrefixes = + new ListPrefixes( + (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); + JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); + + if (matchingFiles.isEmpty()) { + return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); + } + + JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -523,6 +558,31 @@ public Iterator call(Iterator dirs) throws Exception { } } + private static class ListPrefixes implements FlatMapFunction, String> { + + private final SupportsPrefixOperations io; + private final long olderThanTimestamp; + private final Map specs; + + ListPrefixes( + SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { + this.io = io; + this.olderThanTimestamp = olderThanTimestamp; + this.specs = specs; + } + + @Override + public Iterator call(Iterator prefixes) { + Predicate predicate = + file -> file.createdAtMillis() < olderThanTimestamp; + return Iterators.concat( + Iterators.transform( + prefixes, + prefix -> + FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); + } + } + private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index 447f3295c003..0bf19cf91417 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -74,6 +74,10 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); + // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // listing. Only applies when prefix_listing is true. Default is 0 (single seed). + private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = + optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -87,7 +91,8 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM + STREAM_RESULTS_PARAM, + PREFIX_LISTING_MAX_SEED_DEPTH_PARAM }; private static final StructType OUTPUT_TYPE = @@ -143,6 +148,7 @@ public InternalRow[] call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); + int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -191,6 +197,7 @@ public InternalRow[] call(InternalRow args) { } action.usePrefixListing(prefixListing); + action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); diff --git a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index a5ac8a7e01ac..a29a38750837 100644 --- a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -667,6 +667,72 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } + @TestTemplate + public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { + if (catalogName.equals("testhadoop")) { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + } else { + sql( + "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", + tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); + } + + sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); + sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); + + Table table = validationCatalog.loadTable(tableIdent); + String dataLocation = table.location() + "/data"; + + sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); + sql("INSERT INTO TABLE p VALUES (1)"); + sql("INSERT INTO TABLE p VALUES (2)"); + + waitUntilAfter(System.currentTimeMillis()); + + Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); + + List output = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output).as("Should find orphan files in the data folder").hasSize(2); + + List output2 = + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "older_than => TIMESTAMP '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, currentTimestamp, 2); + assertThat(output2).as("Should be no more orphan files").isEmpty(); + + assertEquals( + "Should have expected rows", + ImmutableList.of(row(1L, "a"), row(2L, "b")), + sql("SELECT * FROM %s ORDER BY id", tableName)); + } + + @TestTemplate + public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { + sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); + + assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "prefix_listing => true," + + "prefix_listing_max_seed_depth => %d)", + catalogName, tableIdent, -1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Prefix listing max seed depth must be non-negative"); + } + @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index b47922820d21..2d154a5d965c 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,6 +44,7 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; +import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -131,6 +132,7 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; + private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -222,6 +224,15 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } + public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { + Preconditions.checkArgument( + newMaxSeedDepth >= 0, + "Prefix listing max seed depth must be non-negative: %s", + newMaxSeedDepth); + this.prefixListingMaxSeedDepth = newMaxSeedDepth; + return this; + } + private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -422,17 +433,41 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - matchingFiles::add); + List seedPrefixes = Lists.newArrayList(); + if (prefixListingMaxSeedDepth == 0) { + seedPrefixes.add(location); + } else { + Preconditions.checkArgument( + table.io() instanceof SupportsShallowPrefixOperations, + "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" + + " shallow prefix operations.", + table.io()); + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsShallowPrefixOperations) table.io(), + location, + table.specs(), + predicate, + prefixListingMaxSeedDepth, + MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, + seedPrefixes::add, + matchingFiles::add); + } - JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); + int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); + JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); + ListPrefixes listPrefixes = + new ListPrefixes( + (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); + JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); + + if (matchingFiles.isEmpty()) { + return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); + } + + JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -523,6 +558,31 @@ public Iterator call(Iterator dirs) throws Exception { } } + private static class ListPrefixes implements FlatMapFunction, String> { + + private final SupportsPrefixOperations io; + private final long olderThanTimestamp; + private final Map specs; + + ListPrefixes( + SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { + this.io = io; + this.olderThanTimestamp = olderThanTimestamp; + this.specs = specs; + } + + @Override + public Iterator call(Iterator prefixes) { + Predicate predicate = + file -> file.createdAtMillis() < olderThanTimestamp; + return Iterators.concat( + Iterators.transform( + prefixes, + prefix -> + FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); + } + } + private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index 89e218c10360..e2622f5c9647 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,6 +80,10 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); + // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // listing. Only applies when prefix_listing is true. Default is 0 (single seed). + private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = + optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -93,7 +97,8 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM + STREAM_RESULTS_PARAM, + PREFIX_LISTING_MAX_SEED_DEPTH_PARAM }; private static final StructType OUTPUT_TYPE = @@ -149,6 +154,7 @@ public Iterator call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); + int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -197,6 +203,7 @@ public Iterator call(InternalRow args) { } action.usePrefixListing(prefixListing); + action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); diff --git a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java index 78e8a0b000a4..428613d17a2f 100644 --- a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java +++ b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java @@ -1194,6 +1194,47 @@ public void testRemoveOrphanFileActionWithDeleteMode() { DeleteOrphanFiles.PrefixMismatchMode.DELETE); } + @TestTemplate + public void testPrefixListingMaxSeedDepthDiscoversOrphans() throws IOException { + assumeThat(usePrefixListing) + .as("Seed-depth path is only used when prefix listing is enabled") + .isEqualTo(true); + Table table = TABLES.create(SCHEMA, SPEC, properties, tableLocation); + + List records = + Lists.newArrayList(new ThreeColumnRecord(1, "AAAAAAAAAA", "AAAA")); + Dataset df = spark.createDataFrame(records, ThreeColumnRecord.class).coalesce(1); + df.select("c1", "c2", "c3").write().format("iceberg").mode("append").save(tableLocation); + + // orphan files at multiple depths under the table location + df.coalesce(1).write().mode("append").parquet(tableLocation + "/data/invalid"); + df.coalesce(1).write().mode("append").parquet(tableLocation + "/data/nested/invalid"); + + waitUntilAfter(System.currentTimeMillis()); + + DeleteOrphanFiles.Result result = + SparkActions.get() + .deleteOrphanFiles(table) + .usePrefixListing(true) + .prefixListingMaxSeedDepth(2) + .olderThan(System.currentTimeMillis()) + .execute(); + + assertThat(result.orphanFileLocations()) + .as("Seed-depth prefix listing should still discover orphan files at deeper levels") + .isNotEmpty(); + } + + @TestTemplate + public void testPrefixListingMaxSeedDepthRejectsNegative() { + Table table = TABLES.create(SCHEMA, PartitionSpec.unpartitioned(), properties, tableLocation); + assertThatThrownBy( + () -> + SparkActions.get().deleteOrphanFiles(table).prefixListingMaxSeedDepth(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Prefix listing max seed depth must be non-negative"); + } + @TestTemplate public void testDefaultToHadoopListing() { assumeThat(usePrefixListing) From 00061085eb7f4f403605a93283d515f77c82a2d0 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Fri, 21 Aug 2026 12:43:59 +0100 Subject: [PATCH 04/10] rework APIs --- .../iceberg/encryption/EncryptingFileIO.java | 48 ++++++--------- .../org/apache/iceberg/io/DelegateFileIO.java | 3 +- .../org/apache/iceberg/io/PrefixListing.java | 13 ++-- .../iceberg/io/SupportsPrefixOperations.java | 33 ++++++++++ .../io/SupportsShallowPrefixOperations.java | 38 ------------ .../io/TestSupportsPrefixOperations.java | 61 +++++++++++++++++++ .../org/apache/iceberg/aws/s3/S3FileIO.java | 34 ++++++++++- .../iceberg/azure/adlsv2/TestADLSFileIO.java | 41 +++++++++++++ .../iceberg/azure/adlsv2/ADLSFileIO.java | 17 ++++-- .../apache/iceberg/hadoop/HadoopFileIO.java | 14 ++++- .../apache/iceberg/io/ResolvingFileIO.java | 13 ++-- .../apache/iceberg/util/FileSystemWalker.java | 14 +++-- .../iceberg/hadoop/TestHadoopFileIO.java | 21 +++++-- .../apache/iceberg/io/TestResolvingIO.java | 4 ++ .../iceberg/util/TestFileSystemWalker.java | 33 ++++++++-- .../iceberg/gcp/auth/GoogleAuthSession.java | 2 +- .../org/apache/iceberg/gcp/gcs/GCSFileIO.java | 12 +++- .../actions/DeleteOrphanFilesSparkAction.java | 8 +-- .../actions/DeleteOrphanFilesSparkAction.java | 8 +-- .../actions/DeleteOrphanFilesSparkAction.java | 8 +-- .../actions/TestRemoveOrphanFilesAction.java | 3 +- 21 files changed, 301 insertions(+), 127 deletions(-) delete mode 100644 api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java create mode 100644 api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java diff --git a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java index 601cff3f1992..563b0c2c80a3 100644 --- a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java +++ b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java @@ -37,7 +37,6 @@ import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; @@ -54,8 +53,6 @@ public static EncryptingFileIO combine(FileIO io, EncryptionManager em) { if (io instanceof DelegateFileIO) { return new WithDelegateFileIO((DelegateFileIO) io, em); - } else if (io instanceof SupportsShallowPrefixOperations) { - return new WithSupportsShallowPrefixOperations((SupportsShallowPrefixOperations) io, em); } else if (io instanceof SupportsPrefixOperations) { return new WithSupportsPrefixOperations((SupportsPrefixOperations) io, em); } else { @@ -226,12 +223,12 @@ public EncryptionKeyMetadata copy() { } } - static class WithSupportsShallowPrefixOperations extends EncryptingFileIO - implements SupportsShallowPrefixOperations { + static class WithSupportsPrefixOperations extends EncryptingFileIO + implements SupportsPrefixOperations { - private final SupportsShallowPrefixOperations prefixIo; + private final SupportsPrefixOperations prefixIo; - WithSupportsShallowPrefixOperations(SupportsShallowPrefixOperations io, EncryptionManager em) { + WithSupportsPrefixOperations(SupportsPrefixOperations io, EncryptionManager em) { super(io, em); this.prefixIo = io; } @@ -242,29 +239,13 @@ public Iterable listPrefix(String prefix) { } @Override - public void deletePrefix(String prefix) { - prefixIo.deletePrefix(prefix); + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return prefixIo.supportsPrefixListingWithDelimiter(prefix, delimiter); } @Override - public PrefixListing listImmediate(String prefix) { - return prefixIo.listImmediate(prefix); - } - } - - static class WithSupportsPrefixOperations extends EncryptingFileIO - implements SupportsPrefixOperations { - - private final SupportsPrefixOperations prefixIo; - - WithSupportsPrefixOperations(SupportsPrefixOperations io, EncryptionManager em) { - super(io, em); - this.prefixIo = io; - } - - @Override - public Iterable listPrefix(String prefix) { - return prefixIo.listPrefix(prefix); + public PrefixListing listPrefix(String prefix, String delimiter) { + return prefixIo.listPrefix(prefix, delimiter); } @Override @@ -292,13 +273,18 @@ public Iterable listPrefix(String prefix) { } @Override - public void deletePrefix(String prefix) { - delegateFileIO.deletePrefix(prefix); + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return delegateFileIO.supportsPrefixListingWithDelimiter(prefix, delimiter); } @Override - public PrefixListing listImmediate(String prefix) { - return delegateFileIO.listImmediate(prefix); + public PrefixListing listPrefix(String prefix, String delimiter) { + return delegateFileIO.listPrefix(prefix, delimiter); + } + + @Override + public void deletePrefix(String prefix) { + delegateFileIO.deletePrefix(prefix); } } } diff --git a/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java b/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java index 3199b2952946..4b97b3cb2874 100644 --- a/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java +++ b/api/src/main/java/org/apache/iceberg/io/DelegateFileIO.java @@ -22,5 +22,4 @@ * This interface is intended as an extension for FileIO implementations that support being a * delegate target. */ -public interface DelegateFileIO - extends FileIO, SupportsShallowPrefixOperations, SupportsBulkOperations {} +public interface DelegateFileIO extends FileIO, SupportsPrefixOperations, SupportsBulkOperations {} diff --git a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java index d08ab36bec99..45bdf054df0e 100644 --- a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java +++ b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java @@ -19,8 +19,8 @@ package org.apache.iceberg.io; /** - * Result of a shallow (single-level) prefix listing, separating files found immediately under the - * prefix from sub-prefixes that would be expanded further. + * Result of a delimited prefix listing, separating files from common prefixes that group files + * containing the delimiter. * *

For key/value object stores this maps directly to a delimited listing where {@link #files()} * corresponds to entries returned as objects and {@link #subPrefixes()} corresponds to entries @@ -30,10 +30,15 @@ */ public interface PrefixListing { - /** Files found immediately under the listed prefix. */ + /** Files that do not contain the delimiter after the listed prefix. */ Iterable files(); - /** Sub-prefixes one level below the listed prefix. Each is a location suitable for re-listing. */ + /** + * Common prefixes through the first delimiter after the listed prefix. + * + *

Each common prefix includes the delimiter and is a location suitable for a subsequent + * listing operation. + */ Iterable subPrefixes(); /** Create a {@link PrefixListing} from the given iterables. */ diff --git a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java index e2ebb89146b1..b40f962019ac 100644 --- a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java +++ b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java @@ -35,6 +35,39 @@ public interface SupportsPrefixOperations extends FileIO { */ Iterable listPrefix(String prefix); + /** + * Lists files and common prefixes under a prefix, grouped by a delimiter. + * + *

A file is returned in {@link PrefixListing#files()} when the part of its location after + * {@code prefix} does not contain {@code delimiter}. When the remaining part contains the + * delimiter, the file is not returned directly. Instead, {@link PrefixListing#subPrefixes()} + * contains the common prefix through the first occurrence of the delimiter. Common prefixes are + * unique, include the delimiter, and are suitable for use in a subsequent listing operation. + * + *

Implementations can restrict the supported delimiters. Callers must use {@link + * #supportsPrefixListingWithDelimiter(String, String)} before calling this method. + * + * @param prefix prefix to list + * @param delimiter non-empty delimiter used to group matching locations + * @return files and common prefixes directly below the prefix + * @throws UnsupportedOperationException if prefix listing with the delimiter is not supported + */ + default PrefixListing listPrefix(String prefix, String delimiter) { + throw new UnsupportedOperationException( + String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); + } + + /** + * Returns whether this implementation supports prefix listing with the given delimiter. + * + * @param prefix prefix to list + * @param delimiter non-empty delimiter used to group matching locations + * @return {@code true} if prefix listing with the delimiter is supported + */ + default boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return false; + } + /** * Delete all files under a prefix. * diff --git a/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java b/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java deleted file mode 100644 index 301803117523..000000000000 --- a/api/src/main/java/org/apache/iceberg/io/SupportsShallowPrefixOperations.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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.iceberg.io; - -/** - * Extension of {@link SupportsPrefixOperations} for FileIO implementations that can list a single - * directory level natively (e.g. delimited listing on object stores). - */ -public interface SupportsShallowPrefixOperations extends SupportsPrefixOperations { - - /** - * List the immediate children of a prefix — one directory level down. - * - *

For hierarchical file systems this returns direct file and sub-directory children of the - * given directory. For key/value object stores this maps to a delimited listing where - * sub-prefixes correspond to distinct path segments appearing immediately after the prefix. - * - * @param prefix prefix to list; hierarchical stores may require this to match a directory - * @return files and sub-prefixes found immediately under the given prefix - */ - PrefixListing listImmediate(String prefix); -} diff --git a/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java new file mode 100644 index 000000000000..ac1a0ef90d7f --- /dev/null +++ b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java @@ -0,0 +1,61 @@ +/* + * 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.iceberg.io; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Collections; +import org.junit.jupiter.api.Test; + +class TestSupportsPrefixOperations { + + @Test + void delimitedListingIsUnsupportedByDefault() { + SupportsPrefixOperations io = new TestFileIO(); + + assertThat(io.supportsPrefixListingWithDelimiter("file:/table/", "/")).isFalse(); + assertThatThrownBy(() -> io.listPrefix("file:/table/", "/")) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("Prefix listing with delimiter '/' is not supported"); + } + + private static class TestFileIO implements SupportsPrefixOperations { + @Override + public InputFile newInputFile(String path) { + throw new UnsupportedOperationException(); + } + + @Override + public OutputFile newOutputFile(String path) { + throw new UnsupportedOperationException(); + } + + @Override + public void deleteFile(String path) {} + + @Override + public Iterable listPrefix(String prefix) { + return Collections.emptyList(); + } + + @Override + public void deletePrefix(String prefix) {} + } +} diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java index 633c43b82344..2e957560503f 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java @@ -351,10 +351,15 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listImmediate(String prefix) { + public PrefixListing listPrefix(String prefix, String delimiter) { PrefixedS3Client client = clientForStoragePath(prefix); S3URI uri = new S3URI(prefix, client.s3FileIOProperties().bucketToAccessPointMapping()); + if (!supportsPrefixListingWithDelimiter(uri, client.s3FileIOProperties(), delimiter)) { + throw new UnsupportedOperationException( + String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); + } + if (uri.useS3DirectoryBucket() && client.s3FileIOProperties().isS3DirectoryBucketListPrefixAsDirectory()) { uri = uri.toDirectoryPath(); @@ -365,7 +370,7 @@ public PrefixListing listImmediate(String prefix) { ListObjectsV2Request.builder() .bucket(s3uri.bucket()) .prefix(s3uri.key()) - .delimiter("/") + .delimiter(delimiter) .build(); List files = Lists.newArrayList(); @@ -380,6 +385,31 @@ public PrefixListing listImmediate(String prefix) { return PrefixListing.of(files, subPrefixes); } + @Override + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + if (delimiter == null || delimiter.isEmpty()) { + return false; + } + + PrefixedS3Client client = clientForStoragePath(prefix); + S3URI uri = new S3URI(prefix, client.s3FileIOProperties().bucketToAccessPointMapping()); + return supportsPrefixListingWithDelimiter(uri, client.s3FileIOProperties(), delimiter); + } + + private static boolean supportsPrefixListingWithDelimiter( + S3URI uri, S3FileIOProperties properties, String delimiter) { + if (delimiter == null || delimiter.isEmpty()) { + return false; + } else if (!uri.useS3DirectoryBucket()) { + return true; + } + + return "/".equals(delimiter) + && (uri.key().isEmpty() + || uri.key().endsWith("/") + || properties.isS3DirectoryBucketListPrefixAsDirectory()); + } + private FileInfo createFileInfo(S3URI s3uri, S3Object object) { return new FileInfo( toUri(s3uri, object.key()), object.size(), object.lastModified().toEpochMilli()); diff --git a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java index 45759a1eaebe..4c296b31a9aa 100644 --- a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java +++ b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java @@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -42,6 +43,7 @@ import java.io.OutputStream; import java.time.OffsetDateTime; import java.util.Iterator; +import java.util.function.Consumer; import org.apache.iceberg.TestHelpers; import org.apache.iceberg.azure.AzureProperties; import org.apache.iceberg.exceptions.NotFoundException; @@ -49,6 +51,7 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; @@ -177,6 +180,44 @@ public void testListPrefixOperations() { assertThat(result.hasNext()).isFalse(); } + /** Azurite does not support ADLSv2 directory operations yet so use mocks here. */ + @SuppressWarnings("unchecked") + @Test + void listPrefixWithDelimiter() { + String prefix = "abfs://container@account.dfs.core.windows.net/dir"; + OffsetDateTime now = OffsetDateTime.now(); + PathItem dir = + new PathItem("tag", now, 0L, "group", true, "dir/sub", "owner", "permissions", now, null); + PathItem file = + new PathItem( + "tag", now, 123L, "group", false, "dir/file", "owner", "permissions", now, null); + + PagedIterable response = mock(PagedIterable.class); + doAnswer( + invocation -> { + Consumer consumer = invocation.getArgument(0); + consumer.accept(dir); + consumer.accept(file); + return null; + }) + .when(response) + .forEach(any()); + + DataLakeFileSystemClient client = mock(DataLakeFileSystemClient.class); + when(client.listPaths(any(), any())).thenReturn(response); + + ADLSFileIO io = spy(new ADLSFileIO()); + io.initialize(ImmutableMap.of()); + doReturn(client).when(io).client(any(ADLSLocation.class)); + + PrefixListing listing = io.listPrefix(prefix, "/"); + assertThat(listing.files()) + .extracting(FileInfo::location) + .containsExactly("abfs://container@account.dfs.core.windows.net/dir/file"); + assertThat(listing.subPrefixes()) + .containsExactly("abfs://container@account.dfs.core.windows.net/dir/sub/"); + } + /** Azurite does not support ADLSv2 directory operations yet so use mocks here. */ @SuppressWarnings("unchecked") @Test diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java index 4b0af46a7433..3aca5690f1ce 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java @@ -39,7 +39,6 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.metrics.MetricsContext; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -52,7 +51,7 @@ import org.slf4j.LoggerFactory; /** FileIO implementation backed by Azure Data Lake Storage Gen2. */ -public class ADLSFileIO implements DelegateFileIO, SupportsShallowPrefixOperations { +public class ADLSFileIO implements DelegateFileIO { private static final Logger LOG = LoggerFactory.getLogger(ADLSFileIO.class); private static final String DEFAULT_METRICS_IMPL = @@ -244,7 +243,12 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listImmediate(String prefix) { + public PrefixListing listPrefix(String prefix, String delimiter) { + if (!"/".equals(delimiter)) { + throw new UnsupportedOperationException( + String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); + } + ADLSLocation location = new ADLSLocation(prefix); String baseUri = toBaseUri(location); @@ -261,7 +265,7 @@ public PrefixListing listImmediate(String prefix) { .forEach( pathItem -> { if (pathItem.isDirectory()) { - subPrefixes.add(baseUri + pathItem.getName()); + subPrefixes.add(baseUri + pathItem.getName() + "/"); } else { files.add(createFileInfo(baseUri, pathItem)); } @@ -276,6 +280,11 @@ public PrefixListing listImmediate(String prefix) { return PrefixListing.of(files, subPrefixes); } + @Override + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return "/".equals(delimiter); + } + private String toBaseUri(ADLSLocation location) { return String.format( "%s://%s@%s/", location.scheme(), location.container().orElse(""), location.host()); diff --git a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java index 6d94a34c209b..acbe69e6e8a7 100644 --- a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java +++ b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java @@ -170,7 +170,12 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listImmediate(String prefix) { + public PrefixListing listPrefix(String prefix, String delimiter) { + if (!"/".equals(delimiter)) { + throw new UnsupportedOperationException( + String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); + } + Path prefixToList = new Path(prefix); FileSystem fs = Util.getFs(prefixToList, getConf()); @@ -180,7 +185,7 @@ public PrefixListing listImmediate(String prefix) { for (FileStatus status : fs.listStatus(prefixToList)) { if (status.isDirectory()) { - subPrefixes.add(status.getPath().toString()); + subPrefixes.add(status.getPath() + "/"); } else { files.add(createFileInfo(status)); } @@ -192,6 +197,11 @@ public PrefixListing listImmediate(String prefix) { } } + @Override + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return "/".equals(delimiter); + } + private FileInfo createFileInfo(FileStatus status) { return new FileInfo(status.getPath().toString(), status.getLen(), status.getModificationTime()); } diff --git a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java index 7304f2d479c3..a7be54045070 100644 --- a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java +++ b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java @@ -275,13 +275,18 @@ public Iterable listPrefix(String prefix) { } @Override - public void deletePrefix(String prefix) { - io(prefix).deletePrefix(prefix); + public PrefixListing listPrefix(String prefix, String delimiter) { + return io(prefix).listPrefix(prefix, delimiter); } @Override - public PrefixListing listImmediate(String prefix) { - return io(prefix).listImmediate(prefix); + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return io(prefix).supportsPrefixListingWithDelimiter(prefix, delimiter); + } + + @Override + public void deletePrefix(String prefix) { + io(prefix).deletePrefix(prefix); } @Override diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index 8beb7a308637..54a453938cef 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -38,7 +38,7 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.Iterators; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -105,7 +105,7 @@ public static Iterator listDirRecursivelyWithFileIO( * and adds those sub-prefixes to the pending list. * * - * @param io FileIO implementation that supports shallow prefix listing + * @param io FileIO implementation that supports prefix listing * @param dir the starting prefix to traverse * @param specs partition specs used to preserve partition-name-based hidden paths * @param filter file filter; only files satisfying this condition will be collected @@ -115,7 +115,7 @@ public static Iterator listDirRecursivelyWithFileIO( * @param fileConsumer consumer for qualifying file locations */ public static void listDirRecursivelyWithFileIO( - SupportsShallowPrefixOperations io, + SupportsPrefixOperations io, String dir, Map specs, Predicate filter, @@ -137,7 +137,7 @@ public static void listDirRecursivelyWithFileIO( } private static void listDirRecursivelyWithFileIO( - SupportsShallowPrefixOperations io, + SupportsPrefixOperations io, String baseDir, String dir, PathFilter pathFilter, @@ -152,7 +152,11 @@ private static void listDirRecursivelyWithFileIO( } String listPath = dir.endsWith("/") ? dir : dir + "/"; - PrefixListing listing = io.listImmediate(listPath); + Preconditions.checkArgument( + io.supportsPrefixListingWithDelimiter(listPath, "/"), + "FileIO does not support prefix listing with '/' delimiter: %s", + io); + PrefixListing listing = io.listPrefix(listPath, "/"); List subDirs = Lists.newArrayList(); for (String subPrefix : listing.subPrefixes()) { diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java index f089ceb86b07..27a753f9dfc8 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java @@ -88,7 +88,7 @@ public void testListPrefix() { } @Test - public void testListImmediate() throws IOException { + public void listPrefixWithDelimiter() throws IOException { Path parent = new Path(tempDir.toURI()); Path subDir = new Path(parent, "sub"); @@ -100,14 +100,25 @@ public void testListImmediate() throws IOException { fs.createNewFile(immediateFile1); fs.createNewFile(immediateFile2); - PrefixListing listing = hadoopFileIO.listImmediate(parent.toUri().toString()); + PrefixListing listing = hadoopFileIO.listPrefix(parent.toUri().toString(), "/"); assertThat(listing.files()) .extracting(fileInfo -> new Path(fileInfo.location()).getName()) .containsExactlyInAnyOrder("a.txt", "b.txt"); - assertThat(listing.subPrefixes()) - .extracting(prefix -> new Path(prefix).getName()) - .containsExactly("sub"); + assertThat(listing.subPrefixes()).containsExactly(subDir + "/"); + } + + @Test + void delimitedPrefixListingSupport() { + String prefix = tempDir.toURI().toString(); + + assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, "/")).isTrue(); + assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, "|")).isFalse(); + assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, "")).isFalse(); + assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, null)).isFalse(); + assertThatThrownBy(() -> hadoopFileIO.listPrefix(prefix, "|")) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessage("Prefix listing with delimiter '|' is not supported"); } @Test diff --git a/core/src/test/java/org/apache/iceberg/io/TestResolvingIO.java b/core/src/test/java/org/apache/iceberg/io/TestResolvingIO.java index bc526c18e8cf..9d7d0631b495 100644 --- a/core/src/test/java/org/apache/iceberg/io/TestResolvingIO.java +++ b/core/src/test/java/org/apache/iceberg/io/TestResolvingIO.java @@ -121,6 +121,10 @@ public void delegateFileIOWithPrefixBasedSupport() throws IOException { HadoopFileIO delegate = new HadoopFileIO(hadoopConf); doReturn(delegate).when(resolvingFileIO).io(anyString()); + String parentLocation = parent.toString(); + assertThat(resolvingFileIO.supportsPrefixListingWithDelimiter(parentLocation, "/")).isTrue(); + assertThat(resolvingFileIO.supportsPrefixListingWithDelimiter(parentLocation, "|")).isFalse(); + List paths = IntStream.range(1, 10) .mapToObj(i -> new Path(parent, "random-" + i + "-" + UUID.randomUUID())) diff --git a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java index a66ade3eeb21..cbfd33876d3e 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java +++ b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java @@ -38,7 +38,6 @@ import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -326,8 +325,8 @@ public void testShallowListDirRecursivelyWithFileIOMaxDirectSubDirs() { } @Test - public void testShallowListDirRecursivelyWithFileIONormalizesTrailingSlash() { - RecordingShallowFileIO recordingIO = new RecordingShallowFileIO(); + void delimitedListDirRecursivelyWithFileIONormalizesTrailingSlash() { + RecordingPrefixFileIO recordingIO = new RecordingPrefixFileIO(); FileSystemWalker.listDirRecursivelyWithFileIO( recordingIO, "s3://bucket/table", @@ -343,6 +342,25 @@ public void testShallowListDirRecursivelyWithFileIONormalizesTrailingSlash() { assertThat(recordingIO.calls).containsExactly("s3://bucket/table/"); } + @Test + void rejectsUnsupportedDelimitedPrefixListing() { + SupportsPrefixOperations io = new StaticPrefixFileIO(ImmutableList.of()); + + assertThatThrownBy( + () -> + FileSystemWalker.listDirRecursivelyWithFileIO( + io, + "s3://bucket/table", + null, + fileInfo -> true, + 1, + Integer.MAX_VALUE, + dir -> {}, + file -> {})) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("does not support prefix listing with '/' delimiter"); + } + @Test public void testListDirRecursivelyWithFileIOBucketRootBaseDir() { assertThat( @@ -404,11 +422,16 @@ private static List listWithMockFileIO(String baseDir, FileInfo... entri return foundFiles; } - private static class RecordingShallowFileIO implements SupportsShallowPrefixOperations { + private static class RecordingPrefixFileIO implements SupportsPrefixOperations { private final List calls = Lists.newArrayList(); @Override - public PrefixListing listImmediate(String prefix) { + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return "/".equals(delimiter); + } + + @Override + public PrefixListing listPrefix(String prefix, String delimiter) { calls.add(prefix); return PrefixListing.of(ImmutableList.of(), ImmutableList.of()); } diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java index 14e2b7f0730e..76aa3e14d215 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java @@ -70,7 +70,7 @@ public HTTPRequest authenticate(HTTPRequest request) { HTTPHeaders.of( HTTPHeaders.HTTPHeader.of( "Authorization", "Bearer " + token.getTokenValue()))); - + return newHeaders.equals(request.headers()) ? request : ImmutableHTTPRequest.builder().from(request).headers(newHeaders).build(); diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java index e11b16ca9a5a..0c4fa77f293d 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java @@ -303,7 +303,12 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listImmediate(String prefix) { + public PrefixListing listPrefix(String prefix, String delimiter) { + if (!"/".equals(delimiter)) { + throw new UnsupportedOperationException( + String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); + } + GCSLocation location = new GCSLocation(prefix); List files = Lists.newArrayList(); List subPrefixes = Lists.newArrayList(); @@ -325,6 +330,11 @@ public PrefixListing listImmediate(String prefix) { return PrefixListing.of(files, subPrefixes); } + @Override + public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { + return "/".equals(delimiter); + } + private FileInfo createFileInfo(Blob blob) { return new FileInfo(createUri(blob), blob.getSize(), createTimeMillis(blob)); } diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 2d154a5d965c..9543ebb9be47 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,7 +44,6 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -437,15 +436,10 @@ private Dataset listedFileDS() { if (prefixListingMaxSeedDepth == 0) { seedPrefixes.add(location); } else { - Preconditions.checkArgument( - table.io() instanceof SupportsShallowPrefixOperations, - "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" - + " shallow prefix operations.", - table.io()); Predicate predicate = fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsShallowPrefixOperations) table.io(), + (SupportsPrefixOperations) table.io(), location, table.specs(), predicate, diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 2d154a5d965c..9543ebb9be47 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,7 +44,6 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -437,15 +436,10 @@ private Dataset listedFileDS() { if (prefixListingMaxSeedDepth == 0) { seedPrefixes.add(location); } else { - Preconditions.checkArgument( - table.io() instanceof SupportsShallowPrefixOperations, - "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" - + " shallow prefix operations.", - table.io()); Predicate predicate = fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsShallowPrefixOperations) table.io(), + (SupportsPrefixOperations) table.io(), location, table.specs(), predicate, diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 2d154a5d965c..9543ebb9be47 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -44,7 +44,6 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.SupportsBulkOperations; import org.apache.iceberg.io.SupportsPrefixOperations; -import org.apache.iceberg.io.SupportsShallowPrefixOperations; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -437,15 +436,10 @@ private Dataset listedFileDS() { if (prefixListingMaxSeedDepth == 0) { seedPrefixes.add(location); } else { - Preconditions.checkArgument( - table.io() instanceof SupportsShallowPrefixOperations, - "Cannot use prefix listing seed depth > 0 with FileIO %s which does not support" - + " shallow prefix operations.", - table.io()); Predicate predicate = fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsShallowPrefixOperations) table.io(), + (SupportsPrefixOperations) table.io(), location, table.specs(), predicate, diff --git a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java index 428613d17a2f..80b3b4368397 100644 --- a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java +++ b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java @@ -1229,8 +1229,7 @@ public void testPrefixListingMaxSeedDepthDiscoversOrphans() throws IOException { public void testPrefixListingMaxSeedDepthRejectsNegative() { Table table = TABLES.create(SCHEMA, PartitionSpec.unpartitioned(), properties, tableLocation); assertThatThrownBy( - () -> - SparkActions.get().deleteOrphanFiles(table).prefixListingMaxSeedDepth(-1)) + () -> SparkActions.get().deleteOrphanFiles(table).prefixListingMaxSeedDepth(-1)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Prefix listing max seed depth must be non-negative"); } From 96bc0354ca9ddb7419158e301ad43ece96fb9a6d Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Fri, 21 Aug 2026 13:35:34 +0100 Subject: [PATCH 05/10] Switch to pagination API. --- .../org/apache/iceberg/io/PrefixListing.java | 35 +++------- .../apache/iceberg/io/PrefixListingPage.java | 49 +++++++++++++ .../io/TestSupportsPrefixOperations.java | 13 ++++ .../org/apache/iceberg/aws/s3/S3FileIO.java | 26 ++++--- .../iceberg/azure/adlsv2/TestADLSFileIO.java | 23 +++---- .../iceberg/azure/adlsv2/ADLSFileIO.java | 52 ++++++++------ .../apache/iceberg/hadoop/HadoopFileIO.java | 4 +- .../apache/iceberg/util/FileSystemWalker.java | 19 +++--- .../iceberg/hadoop/TestHadoopFileIO.java | 6 +- .../iceberg/util/TestFileSystemWalker.java | 15 ++-- .../org/apache/iceberg/gcp/gcs/GCSFileIO.java | 68 ++++++++++++++----- 11 files changed, 205 insertions(+), 105 deletions(-) create mode 100644 api/src/main/java/org/apache/iceberg/io/PrefixListingPage.java diff --git a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java index 45bdf054df0e..c577a85359bb 100644 --- a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java +++ b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java @@ -19,39 +19,22 @@ package org.apache.iceberg.io; /** - * Result of a delimited prefix listing, separating files from common prefixes that group files - * containing the delimiter. + * Result of a delimited prefix listing, returned as an iterable of pages. * - *

For key/value object stores this maps directly to a delimited listing where {@link #files()} - * corresponds to entries returned as objects and {@link #subPrefixes()} corresponds to entries - * returned as common prefixes. For hierarchical file systems, {@link #files()} contains regular - * files found directly under the prefix and {@link #subPrefixes()} contains sub-directory - * locations. + *

Pages are retrieved lazily and each page separates files from common prefixes that group files + * containing the delimiter. */ public interface PrefixListing { - /** Files that do not contain the delimiter after the listed prefix. */ - Iterable files(); - - /** - * Common prefixes through the first delimiter after the listed prefix. - * - *

Each common prefix includes the delimiter and is a location suitable for a subsequent - * listing operation. - */ - Iterable subPrefixes(); + /** Pages in this listing. */ + Iterable pages(); - /** Create a {@link PrefixListing} from the given iterables. */ - static PrefixListing of(Iterable files, Iterable subPrefixes) { + /** Create a {@link PrefixListing} from the given pages. */ + static PrefixListing of(Iterable pages) { return new PrefixListing() { @Override - public Iterable files() { - return files; - } - - @Override - public Iterable subPrefixes() { - return subPrefixes; + public Iterable pages() { + return pages; } }; } diff --git a/api/src/main/java/org/apache/iceberg/io/PrefixListingPage.java b/api/src/main/java/org/apache/iceberg/io/PrefixListingPage.java new file mode 100644 index 000000000000..b0a9a04ce510 --- /dev/null +++ b/api/src/main/java/org/apache/iceberg/io/PrefixListingPage.java @@ -0,0 +1,49 @@ +/* + * 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.iceberg.io; + +/** A page in a delimited prefix listing. */ +public interface PrefixListingPage { + + /** Files that do not contain the delimiter after the listed prefix. */ + Iterable files(); + + /** + * Common prefixes through the first delimiter after the listed prefix. + * + *

Each common prefix includes the delimiter and is a location suitable for a subsequent + * listing operation. + */ + Iterable subPrefixes(); + + /** Create a page from the given files and common prefixes. */ + static PrefixListingPage of(Iterable files, Iterable subPrefixes) { + return new PrefixListingPage() { + @Override + public Iterable files() { + return files; + } + + @Override + public Iterable subPrefixes() { + return subPrefixes; + } + }; + } +} diff --git a/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java index ac1a0ef90d7f..d3c6767e2866 100644 --- a/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java +++ b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java @@ -26,6 +26,19 @@ class TestSupportsPrefixOperations { + @Test + void prefixListingContainsPages() { + FileInfo file = new FileInfo("file:/table/file.parquet", 10L, 20L); + PrefixListingPage page = + PrefixListingPage.of( + Collections.singletonList(file), Collections.singletonList("file:/table/partition/")); + PrefixListing listing = PrefixListing.of(Collections.singletonList(page)); + + assertThat(listing.pages()).containsExactly(page); + assertThat(page.files()).containsExactly(file); + assertThat(page.subPrefixes()).containsExactly("file:/table/partition/"); + } + @Test void delimitedListingIsUnsupportedByDefault() { SupportsPrefixOperations io = new TestFileIO(); diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java index 2e957560503f..cd92a7105b02 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java @@ -45,6 +45,7 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsRecoveryOperations; import org.apache.iceberg.io.SupportsStorageCredentials; @@ -373,16 +374,11 @@ public PrefixListing listPrefix(String prefix, String delimiter) { .delimiter(delimiter) .build(); - List files = Lists.newArrayList(); - List subPrefixes = Lists.newArrayList(); - for (ListObjectsV2Response response : client.s3().listObjectsV2Paginator(request)) { - response.contents().forEach(o -> files.add(createFileInfo(s3uri, o))); - response - .commonPrefixes() - .forEach(commonPrefix -> subPrefixes.add(toUri(s3uri, commonPrefix.prefix()))); - } - - return PrefixListing.of(files, subPrefixes); + return PrefixListing.of( + () -> + client.s3().listObjectsV2Paginator(request).stream() + .map(response -> createPrefixListingPage(s3uri, response)) + .iterator()); } @Override @@ -410,6 +406,16 @@ private static boolean supportsPrefixListingWithDelimiter( || properties.isS3DirectoryBucketListPrefixAsDirectory()); } + private PrefixListingPage createPrefixListingPage(S3URI s3uri, ListObjectsV2Response response) { + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + response.contents().forEach(object -> files.add(createFileInfo(s3uri, object))); + response + .commonPrefixes() + .forEach(commonPrefix -> subPrefixes.add(toUri(s3uri, commonPrefix.prefix()))); + return PrefixListingPage.of(files, subPrefixes); + } + private FileInfo createFileInfo(S3URI s3uri, S3Object object) { return new FileInfo( toUri(s3uri, object.key()), object.size(), object.lastModified().toEpochMilli()); diff --git a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java index 4c296b31a9aa..6f4460369665 100644 --- a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java +++ b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java @@ -24,7 +24,6 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -32,7 +31,9 @@ import static org.mockito.Mockito.when; import com.azure.core.http.rest.PagedIterable; +import com.azure.core.http.rest.PagedResponse; import com.azure.core.http.rest.Response; +import com.azure.core.util.IterableStream; import com.azure.storage.blob.models.BlobStorageException; import com.azure.storage.file.datalake.DataLakeFileClient; import com.azure.storage.file.datalake.DataLakeFileSystemClient; @@ -43,7 +44,6 @@ import java.io.OutputStream; import java.time.OffsetDateTime; import java.util.Iterator; -import java.util.function.Consumer; import org.apache.iceberg.TestHelpers; import org.apache.iceberg.azure.AzureProperties; import org.apache.iceberg.exceptions.NotFoundException; @@ -52,6 +52,7 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; @@ -193,15 +194,10 @@ void listPrefixWithDelimiter() { "tag", now, 123L, "group", false, "dir/file", "owner", "permissions", now, null); PagedIterable response = mock(PagedIterable.class); - doAnswer( - invocation -> { - Consumer consumer = invocation.getArgument(0); - consumer.accept(dir); - consumer.accept(file); - return null; - }) - .when(response) - .forEach(any()); + PagedResponse pageResponse = mock(PagedResponse.class); + when(pageResponse.getElements()).thenReturn(new IterableStream<>(ImmutableList.of(dir, file))); + when(response.iterableByPage()) + .thenReturn(new IterableStream<>(ImmutableList.of(pageResponse))); DataLakeFileSystemClient client = mock(DataLakeFileSystemClient.class); when(client.listPaths(any(), any())).thenReturn(response); @@ -211,10 +207,11 @@ void listPrefixWithDelimiter() { doReturn(client).when(io).client(any(ADLSLocation.class)); PrefixListing listing = io.listPrefix(prefix, "/"); - assertThat(listing.files()) + PrefixListingPage page = listing.pages().iterator().next(); + assertThat(page.files()) .extracting(FileInfo::location) .containsExactly("abfs://container@account.dfs.core.windows.net/dir/file"); - assertThat(listing.subPrefixes()) + assertThat(page.subPrefixes()) .containsExactly("abfs://container@account.dfs.core.windows.net/dir/sub/"); } diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java index 3aca5690f1ce..504a2431d73c 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java @@ -39,10 +39,12 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.metrics.MetricsContext; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.relocated.com.google.common.collect.Streams; import org.apache.iceberg.util.SerializableFunction; import org.apache.iceberg.util.SerializableMap; import org.apache.iceberg.util.Tasks; @@ -256,28 +258,20 @@ public PrefixListing listPrefix(String prefix, String delimiter) { options.setPath(location.path()); options.setRecursive(false); - List files = Lists.newArrayList(); - List subPrefixes = Lists.newArrayList(); - - try { - client(location) - .listPaths(options, null) - .forEach( - pathItem -> { - if (pathItem.isDirectory()) { - subPrefixes.add(baseUri + pathItem.getName() + "/"); - } else { - files.add(createFileInfo(baseUri, pathItem)); - } - }); - } catch (DataLakeStorageException e) { - if (e.getStatusCode() != 404) { - throw e; - } - return PrefixListing.of(Collections.emptyList(), Collections.emptyList()); - } - - return PrefixListing.of(files, subPrefixes); + return PrefixListing.of( + () -> { + try { + return Streams.stream(client(location).listPaths(options, null).iterableByPage()) + .map(response -> createPrefixListingPage(baseUri, response.getElements())) + .iterator(); + } catch (DataLakeStorageException e) { + if (e.getStatusCode() != 404) { + throw e; + } + + return Collections.emptyIterator(); + } + }); } @Override @@ -297,6 +291,20 @@ private FileInfo createFileInfo(String baseUri, PathItem pathItem) { pathItem.getCreationTime().toInstant().toEpochMilli()); } + private PrefixListingPage createPrefixListingPage(String baseUri, Iterable pathItems) { + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + for (PathItem pathItem : pathItems) { + if (pathItem.isDirectory()) { + subPrefixes.add(baseUri + pathItem.getName() + "/"); + } else { + files.add(createFileInfo(baseUri, pathItem)); + } + } + + return PrefixListingPage.of(files, subPrefixes); + } + @Override public void deletePrefix(String prefix) { ADLSLocation location = new ADLSLocation(prefix); diff --git a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java index acbe69e6e8a7..6c5850b0f5e9 100644 --- a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java +++ b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java @@ -21,6 +21,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -39,6 +40,7 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Streams; @@ -191,7 +193,7 @@ public PrefixListing listPrefix(String prefix, String delimiter) { } } - return PrefixListing.of(files, subPrefixes); + return PrefixListing.of(Collections.singletonList(PrefixListingPage.of(files, subPrefixes))); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index 54a453938cef..bafcfb6373e0 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -37,6 +37,7 @@ import org.apache.iceberg.hadoop.HiddenPathFilter; import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.Iterators; @@ -159,16 +160,18 @@ private static void listDirRecursivelyWithFileIO( PrefixListing listing = io.listPrefix(listPath, "/"); List subDirs = Lists.newArrayList(); - for (String subPrefix : listing.subPrefixes()) { - if (!isHiddenPath(baseDir, new Path(subPrefix), pathFilter)) { - subDirs.add(subPrefix); + for (PrefixListingPage page : listing.pages()) { + for (String subPrefix : page.subPrefixes()) { + if (!isHiddenPath(baseDir, new Path(subPrefix), pathFilter)) { + subDirs.add(subPrefix); + } } - } - for (FileInfo file : listing.files()) { - Path filePath = new Path(file.location()); - if (!isHiddenPath(baseDir, filePath, pathFilter) && filter.test(file)) { - fileConsumer.accept(file.location()); + for (FileInfo file : page.files()) { + Path filePath = new Path(file.location()); + if (!isHiddenPath(baseDir, filePath, pathFilter) && filter.test(file)) { + fileConsumer.accept(file.location()); + } } } diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java index 27a753f9dfc8..273126de46e2 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java @@ -39,6 +39,7 @@ import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.FileIOParser; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -101,11 +102,12 @@ public void listPrefixWithDelimiter() throws IOException { fs.createNewFile(immediateFile2); PrefixListing listing = hadoopFileIO.listPrefix(parent.toUri().toString(), "/"); + PrefixListingPage page = listing.pages().iterator().next(); - assertThat(listing.files()) + assertThat(page.files()) .extracting(fileInfo -> new Path(fileInfo.location()).getName()) .containsExactlyInAnyOrder("a.txt", "b.txt"); - assertThat(listing.subPrefixes()).containsExactly(subDir + "/"); + assertThat(page.subPrefixes()).containsExactly(subDir + "/"); } @Test diff --git a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java index cbfd33876d3e..ed70ee629f6e 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java +++ b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java @@ -37,6 +37,7 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -298,7 +299,7 @@ public void testShallowListDirRecursivelyWithFileIOMaxDepth() { .contains(Paths.get("file://", basePath, "normal_dir_1/file4.txt").toString()); assertThat(remainingDirs).hasSize(1); assertThat(remainingDirs) - .contains(Paths.get("file://", basePath, "normal_dir/dep1").toString()); + .contains(Paths.get("file://", basePath, "normal_dir/dep1").toString() + "/"); } @Test @@ -319,9 +320,12 @@ public void testShallowListDirRecursivelyWithFileIOMaxDirectSubDirs() { assertThat(foundFiles).hasSize(1); assertThat(foundFiles).contains(Paths.get("file://", basePath, "file1.txt").toString()); assertThat(remainingDirs).hasSize(3); - assertThat(remainingDirs).contains(Paths.get("file://", basePath, "normal_dir").toString()); - assertThat(remainingDirs).contains(Paths.get("file://", basePath, "normal_dir_1").toString()); - assertThat(remainingDirs).contains(Paths.get("file://", basePath, "hidden_dir").toString()); + assertThat(remainingDirs) + .contains(Paths.get("file://", basePath, "normal_dir").toString() + "/"); + assertThat(remainingDirs) + .contains(Paths.get("file://", basePath, "normal_dir_1").toString() + "/"); + assertThat(remainingDirs) + .contains(Paths.get("file://", basePath, "hidden_dir").toString() + "/"); } @Test @@ -433,7 +437,8 @@ public boolean supportsPrefixListingWithDelimiter(String prefix, String delimite @Override public PrefixListing listPrefix(String prefix, String delimiter) { calls.add(prefix); - return PrefixListing.of(ImmutableList.of(), ImmutableList.of()); + return PrefixListing.of( + ImmutableList.of(PrefixListingPage.of(ImmutableList.of(), ImmutableList.of()))); } @Override diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java index 0c4fa77f293d..109137134409 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java @@ -20,6 +20,7 @@ import com.google.api.client.util.Lists; import com.google.api.client.util.Maps; +import com.google.api.gax.paging.Page; import com.google.cloud.storage.Blob; import com.google.cloud.storage.BlobId; import com.google.cloud.storage.Storage; @@ -27,8 +28,10 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -44,6 +47,7 @@ import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsStorageCredentials; import org.apache.iceberg.metrics.MetricsContext; @@ -310,24 +314,15 @@ public PrefixListing listPrefix(String prefix, String delimiter) { } GCSLocation location = new GCSLocation(prefix); - List files = Lists.newArrayList(); - List subPrefixes = Lists.newArrayList(); - clientForStoragePath(prefix) - .storage() - .list( - location.bucket(), - Storage.BlobListOption.prefix(location.prefix()), - Storage.BlobListOption.currentDirectory()) - .streamAll() - .forEach( - blob -> { - if (blob.isDirectory()) { - subPrefixes.add(createUri(blob)); - } else { - files.add(createFileInfo(blob)); - } - }); - return PrefixListing.of(files, subPrefixes); + return PrefixListing.of( + () -> + prefixListingPages( + clientForStoragePath(prefix) + .storage() + .list( + location.bucket(), + Storage.BlobListOption.prefix(location.prefix()), + Storage.BlobListOption.currentDirectory()))); } @Override @@ -339,6 +334,43 @@ private FileInfo createFileInfo(Blob blob) { return new FileInfo(createUri(blob), blob.getSize(), createTimeMillis(blob)); } + private Iterator prefixListingPages(Page firstPage) { + return new Iterator() { + private Page currentPage = firstPage; + private boolean first = true; + + @Override + public boolean hasNext() { + return first || currentPage.hasNextPage(); + } + + @Override + public PrefixListingPage next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + + if (first) { + first = false; + } else { + currentPage = currentPage.getNextPage(); + } + + List files = Lists.newArrayList(); + List subPrefixes = Lists.newArrayList(); + for (Blob blob : currentPage.getValues()) { + if (blob.isDirectory()) { + subPrefixes.add(createUri(blob)); + } else { + files.add(createFileInfo(blob)); + } + } + + return PrefixListingPage.of(files, subPrefixes); + } + }; + } + private String createUri(Blob blob) { return String.format("gs://%s/%s", blob.getBucket(), blob.getName()); } From c5085d77c719e6c3468c7f49689a100f52f049ac Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Mon, 24 Aug 2026 12:56:52 +0100 Subject: [PATCH 06/10] update --- .../java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java | 1 - .../iceberg/spark/procedures/RemoveOrphanFilesProcedure.java | 2 +- .../iceberg/spark/procedures/RemoveOrphanFilesProcedure.java | 2 +- .../iceberg/spark/procedures/RemoveOrphanFilesProcedure.java | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java index 76aa3e14d215..c81053613da4 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/auth/GoogleAuthSession.java @@ -70,7 +70,6 @@ public HTTPRequest authenticate(HTTPRequest request) { HTTPHeaders.of( HTTPHeaders.HTTPHeader.of( "Authorization", "Bearer " + token.getTokenValue()))); - return newHeaders.equals(request.headers()) ? request : ImmutableHTTPRequest.builder().from(request).headers(newHeaders).build(); diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index 0bf19cf91417..337d5e2fedb9 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -74,7 +74,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // Depth of driver-side listing used to discover seed prefixes for parallel prefix // listing. Only applies when prefix_listing is true. Default is 0 (single seed). private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index e2622f5c9647..f9dd6093cca1 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,7 +80,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // Depth of driver-side listing used to discover seed prefixes for parallel prefix // listing. Only applies when prefix_listing is true. Default is 0 (single seed). private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index e2622f5c9647..f9dd6093cca1 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,7 +80,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of shallow driver-side walk used to discover seed prefixes for parallel prefix + // Depth of driver-side listing used to discover seed prefixes for parallel prefix // listing. Only applies when prefix_listing is true. Default is 0 (single seed). private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); From 7f9fec834901113d82235ba6033654a682296fe6 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Mon, 24 Aug 2026 13:12:36 +0100 Subject: [PATCH 07/10] fix javadoc --- .../java/org/apache/iceberg/io/SupportsPrefixOperations.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java index b40f962019ac..679aeb054149 100644 --- a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java +++ b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java @@ -38,9 +38,9 @@ public interface SupportsPrefixOperations extends FileIO { /** * Lists files and common prefixes under a prefix, grouped by a delimiter. * - *

A file is returned in {@link PrefixListing#files()} when the part of its location after + *

A file is returned in {@link PrefixListingPage#files()} when the part of its location after * {@code prefix} does not contain {@code delimiter}. When the remaining part contains the - * delimiter, the file is not returned directly. Instead, {@link PrefixListing#subPrefixes()} + * delimiter, the file is not returned directly. Instead, {@link PrefixListingPage#subPrefixes()} * contains the common prefix through the first occurrence of the delimiter. Common prefixes are * unique, include the delimiter, and are suitable for use in a subsequent listing operation. * From 599dc9c9dbd4d621cafd4def95949f68dd0af37d Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Tue, 8 Sep 2026 12:50:37 +0100 Subject: [PATCH 08/10] streaming --- .../apache/iceberg/util/FileSystemWalker.java | 15 ++++- .../iceberg/util/TestFileSystemWalker.java | 62 ++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index bafcfb6373e0..5ff7d552e8cb 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -160,10 +160,20 @@ private static void listDirRecursivelyWithFileIO( PrefixListing listing = io.listPrefix(listPath, "/"); List subDirs = Lists.newArrayList(); + boolean maxDirectSubDirsExceeded = false; for (PrefixListingPage page : listing.pages()) { for (String subPrefix : page.subPrefixes()) { if (!isHiddenPath(baseDir, new Path(subPrefix), pathFilter)) { - subDirs.add(subPrefix); + if (maxDirectSubDirsExceeded) { + directoryConsumer.accept(subPrefix); + } else if (subDirs.size() >= maxDirectSubDirs) { + maxDirectSubDirsExceeded = true; + subDirs.forEach(directoryConsumer); + subDirs.clear(); + directoryConsumer.accept(subPrefix); + } else { + subDirs.add(subPrefix); + } } } @@ -175,8 +185,7 @@ private static void listDirRecursivelyWithFileIO( } } - if (subDirs.size() > maxDirectSubDirs) { - subDirs.forEach(directoryConsumer); + if (maxDirectSubDirsExceeded) { return; } diff --git a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java index ed70ee629f6e..a64958b832a9 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java +++ b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java @@ -25,6 +25,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Predicate; @@ -328,6 +329,53 @@ public void testShallowListDirRecursivelyWithFileIOMaxDirectSubDirs() { .contains(Paths.get("file://", basePath, "hidden_dir").toString() + "/"); } + @Test + void streamsSubPrefixesAfterMaxDirectSubDirsExceeded() { + List remainingDirs = Lists.newArrayList(); + PrefixListingPage firstPage = + PrefixListingPage.of( + ImmutableList.of(), ImmutableList.of("s3://bucket/table/a/", "s3://bucket/table/b/")); + PrefixListingPage secondPage = + PrefixListingPage.of(ImmutableList.of(), ImmutableList.of("s3://bucket/table/c/")); + Iterable pages = + () -> + new Iterator() { + private final Iterator delegate = + ImmutableList.of(firstPage, secondPage).iterator(); + private boolean first = true; + + @Override + public boolean hasNext() { + return delegate.hasNext(); + } + + @Override + public PrefixListingPage next() { + if (!first) { + assertThat(remainingDirs) + .containsExactly("s3://bucket/table/a/", "s3://bucket/table/b/"); + } + + first = false; + return delegate.next(); + } + }; + RecordingPrefixFileIO recordingIO = new RecordingPrefixFileIO(PrefixListing.of(pages)); + + FileSystemWalker.listDirRecursivelyWithFileIO( + recordingIO, + "s3://bucket/table/", + null, + fileInfo -> true, + 1, + 1, + remainingDirs::add, + file -> {}); + + assertThat(remainingDirs) + .containsExactly("s3://bucket/table/a/", "s3://bucket/table/b/", "s3://bucket/table/c/"); + } + @Test void delimitedListDirRecursivelyWithFileIONormalizesTrailingSlash() { RecordingPrefixFileIO recordingIO = new RecordingPrefixFileIO(); @@ -428,6 +476,17 @@ private static List listWithMockFileIO(String baseDir, FileInfo... entri private static class RecordingPrefixFileIO implements SupportsPrefixOperations { private final List calls = Lists.newArrayList(); + private final PrefixListing listing; + + private RecordingPrefixFileIO() { + this( + PrefixListing.of( + ImmutableList.of(PrefixListingPage.of(ImmutableList.of(), ImmutableList.of())))); + } + + private RecordingPrefixFileIO(PrefixListing listing) { + this.listing = listing; + } @Override public boolean supportsPrefixListingWithDelimiter(String prefix, String delimiter) { @@ -437,8 +496,7 @@ public boolean supportsPrefixListingWithDelimiter(String prefix, String delimite @Override public PrefixListing listPrefix(String prefix, String delimiter) { calls.add(prefix); - return PrefixListing.of( - ImmutableList.of(PrefixListingPage.of(ImmutableList.of(), ImmutableList.of()))); + return listing; } @Override From e1ffb8ab937e18f504239b869d3dcbbb209aca59 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Tue, 6 Oct 2026 21:44:52 +0100 Subject: [PATCH 09/10] Remove Spark changes --- .../iceberg/encryption/EncryptingFileIO.java | 10 +-- .../org/apache/iceberg/io/PrefixListing.java | 41 --------- .../iceberg/io/SupportsPrefixOperations.java | 8 +- .../io/TestSupportsPrefixOperations.java | 6 +- .../org/apache/iceberg/aws/s3/S3FileIO.java | 12 ++- .../iceberg/azure/adlsv2/TestADLSFileIO.java | 5 +- .../iceberg/azure/adlsv2/ADLSFileIO.java | 30 ++++--- .../apache/iceberg/hadoop/HadoopFileIO.java | 5 +- .../apache/iceberg/io/ResolvingFileIO.java | 4 +- .../apache/iceberg/util/FileSystemWalker.java | 7 +- .../iceberg/hadoop/TestHadoopFileIO.java | 8 +- .../iceberg/util/TestFileSystemWalker.java | 17 ++-- .../org/apache/iceberg/gcp/gcs/GCSFileIO.java | 20 ++--- .../TestRemoveOrphanFilesProcedure.java | 66 --------------- .../actions/DeleteOrphanFilesSparkAction.java | 74 +++------------- .../RemoveOrphanFilesProcedure.java | 9 +- .../TestRemoveOrphanFilesProcedure.java | 66 --------------- .../actions/DeleteOrphanFilesSparkAction.java | 74 +++------------- .../RemoveOrphanFilesProcedure.java | 9 +- .../TestRemoveOrphanFilesProcedure.java | 84 ------------------- .../actions/DeleteOrphanFilesSparkAction.java | 74 +++------------- .../RemoveOrphanFilesProcedure.java | 9 +- .../actions/TestRemoveOrphanFilesAction.java | 40 --------- 23 files changed, 95 insertions(+), 583 deletions(-) delete mode 100644 api/src/main/java/org/apache/iceberg/io/PrefixListing.java diff --git a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java index 563b0c2c80a3..a7be0aa90b7a 100644 --- a/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java +++ b/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java @@ -35,7 +35,7 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; +import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; @@ -244,8 +244,8 @@ public boolean supportsPrefixListingWithDelimiter(String prefix, String delimite } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { - return prefixIo.listPrefix(prefix, delimiter); + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { + return prefixIo.listPrefixWithDelimiter(prefix, delimiter); } @Override @@ -278,8 +278,8 @@ public boolean supportsPrefixListingWithDelimiter(String prefix, String delimite } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { - return delegateFileIO.listPrefix(prefix, delimiter); + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { + return delegateFileIO.listPrefixWithDelimiter(prefix, delimiter); } @Override diff --git a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java b/api/src/main/java/org/apache/iceberg/io/PrefixListing.java deleted file mode 100644 index c577a85359bb..000000000000 --- a/api/src/main/java/org/apache/iceberg/io/PrefixListing.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * 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.iceberg.io; - -/** - * Result of a delimited prefix listing, returned as an iterable of pages. - * - *

Pages are retrieved lazily and each page separates files from common prefixes that group files - * containing the delimiter. - */ -public interface PrefixListing { - - /** Pages in this listing. */ - Iterable pages(); - - /** Create a {@link PrefixListing} from the given pages. */ - static PrefixListing of(Iterable pages) { - return new PrefixListing() { - @Override - public Iterable pages() { - return pages; - } - }; - } -} diff --git a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java index 679aeb054149..c8de89ecf914 100644 --- a/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java +++ b/api/src/main/java/org/apache/iceberg/io/SupportsPrefixOperations.java @@ -49,10 +49,10 @@ public interface SupportsPrefixOperations extends FileIO { * * @param prefix prefix to list * @param delimiter non-empty delimiter used to group matching locations - * @return files and common prefixes directly below the prefix + * @return iterable of pages containing files and common prefixes directly below the prefix * @throws UnsupportedOperationException if prefix listing with the delimiter is not supported */ - default PrefixListing listPrefix(String prefix, String delimiter) { + default Iterable listPrefixWithDelimiter(String prefix, String delimiter) { throw new UnsupportedOperationException( String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); } @@ -60,6 +60,10 @@ default PrefixListing listPrefix(String prefix, String delimiter) { /** * Returns whether this implementation supports prefix listing with the given delimiter. * + *

Support can vary by prefix when a FileIO selects a storage implementation from the location + * or when the target has additional restrictions, such as an S3 directory bucket. Callers must + * check support for each prefix and delimiter pair they intend to list. + * * @param prefix prefix to list * @param delimiter non-empty delimiter used to group matching locations * @return {@code true} if prefix listing with the delimiter is supported diff --git a/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java index d3c6767e2866..2fab3e9d3165 100644 --- a/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java +++ b/api/src/test/java/org/apache/iceberg/io/TestSupportsPrefixOperations.java @@ -27,14 +27,12 @@ class TestSupportsPrefixOperations { @Test - void prefixListingContainsPages() { + void prefixListingPageContainsFilesAndSubPrefixes() { FileInfo file = new FileInfo("file:/table/file.parquet", 10L, 20L); PrefixListingPage page = PrefixListingPage.of( Collections.singletonList(file), Collections.singletonList("file:/table/partition/")); - PrefixListing listing = PrefixListing.of(Collections.singletonList(page)); - assertThat(listing.pages()).containsExactly(page); assertThat(page.files()).containsExactly(file); assertThat(page.subPrefixes()).containsExactly("file:/table/partition/"); } @@ -44,7 +42,7 @@ void delimitedListingIsUnsupportedByDefault() { SupportsPrefixOperations io = new TestFileIO(); assertThat(io.supportsPrefixListingWithDelimiter("file:/table/", "/")).isFalse(); - assertThatThrownBy(() -> io.listPrefix("file:/table/", "/")) + assertThatThrownBy(() -> io.listPrefixWithDelimiter("file:/table/", "/")) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Prefix listing with delimiter '/' is not supported"); } diff --git a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java index cd92a7105b02..d62a3136f479 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java +++ b/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java @@ -44,7 +44,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsRecoveryOperations; @@ -352,7 +351,7 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { PrefixedS3Client client = clientForStoragePath(prefix); S3URI uri = new S3URI(prefix, client.s3FileIOProperties().bucketToAccessPointMapping()); @@ -374,11 +373,10 @@ public PrefixListing listPrefix(String prefix, String delimiter) { .delimiter(delimiter) .build(); - return PrefixListing.of( - () -> - client.s3().listObjectsV2Paginator(request).stream() - .map(response -> createPrefixListingPage(s3uri, response)) - .iterator()); + return () -> + client.s3().listObjectsV2Paginator(request).stream() + .map(response -> createPrefixListingPage(s3uri, response)) + .iterator(); } @Override diff --git a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java index 6f4460369665..0c109c51424e 100644 --- a/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java +++ b/azure/src/integration/java/org/apache/iceberg/azure/adlsv2/TestADLSFileIO.java @@ -51,7 +51,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -206,8 +205,8 @@ void listPrefixWithDelimiter() { io.initialize(ImmutableMap.of()); doReturn(client).when(io).client(any(ADLSLocation.class)); - PrefixListing listing = io.listPrefix(prefix, "/"); - PrefixListingPage page = listing.pages().iterator().next(); + Iterable listing = io.listPrefixWithDelimiter(prefix, "/"); + PrefixListingPage page = listing.iterator().next(); assertThat(page.files()) .extracting(FileInfo::location) .containsExactly("abfs://container@account.dfs.core.windows.net/dir/file"); diff --git a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java index 504a2431d73c..c9aef932d804 100644 --- a/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java +++ b/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java @@ -38,7 +38,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.metrics.MetricsContext; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; @@ -245,7 +244,7 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { if (!"/".equals(delimiter)) { throw new UnsupportedOperationException( String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); @@ -258,20 +257,19 @@ public PrefixListing listPrefix(String prefix, String delimiter) { options.setPath(location.path()); options.setRecursive(false); - return PrefixListing.of( - () -> { - try { - return Streams.stream(client(location).listPaths(options, null).iterableByPage()) - .map(response -> createPrefixListingPage(baseUri, response.getElements())) - .iterator(); - } catch (DataLakeStorageException e) { - if (e.getStatusCode() != 404) { - throw e; - } - - return Collections.emptyIterator(); - } - }); + return () -> { + try { + return Streams.stream(client(location).listPaths(options, null).iterableByPage()) + .map(response -> createPrefixListingPage(baseUri, response.getElements())) + .iterator(); + } catch (DataLakeStorageException e) { + if (e.getStatusCode() != 404) { + throw e; + } + + return Collections.emptyIterator(); + } + }; } @Override diff --git a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java index 6c5850b0f5e9..aa0bacefe080 100644 --- a/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java +++ b/core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java @@ -39,7 +39,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -172,7 +171,7 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { if (!"/".equals(delimiter)) { throw new UnsupportedOperationException( String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); @@ -193,7 +192,7 @@ public PrefixListing listPrefix(String prefix, String delimiter) { } } - return PrefixListing.of(Collections.singletonList(PrefixListingPage.of(files, subPrefixes))); + return Collections.singletonList(PrefixListingPage.of(files, subPrefixes)); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java index a7be54045070..ffa55cf0cdf5 100644 --- a/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java +++ b/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java @@ -275,8 +275,8 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { - return io(prefix).listPrefix(prefix, delimiter); + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { + return io(prefix).listPrefixWithDelimiter(prefix, delimiter); } @Override diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index 5ff7d552e8cb..4055bd87f296 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -36,7 +36,6 @@ import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.hadoop.HiddenPathFilter; import org.apache.iceberg.io.FileInfo; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; @@ -156,12 +155,12 @@ private static void listDirRecursivelyWithFileIO( Preconditions.checkArgument( io.supportsPrefixListingWithDelimiter(listPath, "/"), "FileIO does not support prefix listing with '/' delimiter: %s", - io); - PrefixListing listing = io.listPrefix(listPath, "/"); + io.getClass().getName()); + Iterable listing = io.listPrefixWithDelimiter(listPath, "/"); List subDirs = Lists.newArrayList(); boolean maxDirectSubDirsExceeded = false; - for (PrefixListingPage page : listing.pages()) { + for (PrefixListingPage page : listing) { for (String subPrefix : page.subPrefixes()) { if (!isHiddenPath(baseDir, new Path(subPrefix), pathFilter)) { if (maxDirectSubDirsExceeded) { diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java index 273126de46e2..86727edf1cc3 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopFileIO.java @@ -38,7 +38,6 @@ import org.apache.iceberg.io.BulkDeletionFailureException; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.FileIOParser; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.ResolvingFileIO; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -101,8 +100,9 @@ public void listPrefixWithDelimiter() throws IOException { fs.createNewFile(immediateFile1); fs.createNewFile(immediateFile2); - PrefixListing listing = hadoopFileIO.listPrefix(parent.toUri().toString(), "/"); - PrefixListingPage page = listing.pages().iterator().next(); + Iterable listing = + hadoopFileIO.listPrefixWithDelimiter(parent.toUri().toString(), "/"); + PrefixListingPage page = listing.iterator().next(); assertThat(page.files()) .extracting(fileInfo -> new Path(fileInfo.location()).getName()) @@ -118,7 +118,7 @@ void delimitedPrefixListingSupport() { assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, "|")).isFalse(); assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, "")).isFalse(); assertThat(hadoopFileIO.supportsPrefixListingWithDelimiter(prefix, null)).isFalse(); - assertThatThrownBy(() -> hadoopFileIO.listPrefix(prefix, "|")) + assertThatThrownBy(() -> hadoopFileIO.listPrefixWithDelimiter(prefix, "|")) .isInstanceOf(UnsupportedOperationException.class) .hasMessage("Prefix listing with delimiter '|' is not supported"); } diff --git a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java index a64958b832a9..40168bfe7345 100644 --- a/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java +++ b/core/src/test/java/org/apache/iceberg/util/TestFileSystemWalker.java @@ -37,7 +37,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -360,7 +359,7 @@ public PrefixListingPage next() { return delegate.next(); } }; - RecordingPrefixFileIO recordingIO = new RecordingPrefixFileIO(PrefixListing.of(pages)); + RecordingPrefixFileIO recordingIO = new RecordingPrefixFileIO(pages); FileSystemWalker.listDirRecursivelyWithFileIO( recordingIO, @@ -410,7 +409,9 @@ void rejectsUnsupportedDelimitedPrefixListing() { dir -> {}, file -> {})) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("does not support prefix listing with '/' delimiter"); + .hasMessage( + "FileIO does not support prefix listing with '/' delimiter: %s", + StaticPrefixFileIO.class.getName()); } @Test @@ -476,15 +477,13 @@ private static List listWithMockFileIO(String baseDir, FileInfo... entri private static class RecordingPrefixFileIO implements SupportsPrefixOperations { private final List calls = Lists.newArrayList(); - private final PrefixListing listing; + private final Iterable listing; private RecordingPrefixFileIO() { - this( - PrefixListing.of( - ImmutableList.of(PrefixListingPage.of(ImmutableList.of(), ImmutableList.of())))); + this(ImmutableList.of(PrefixListingPage.of(ImmutableList.of(), ImmutableList.of()))); } - private RecordingPrefixFileIO(PrefixListing listing) { + private RecordingPrefixFileIO(Iterable listing) { this.listing = listing; } @@ -494,7 +493,7 @@ public boolean supportsPrefixListingWithDelimiter(String prefix, String delimite } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { calls.add(prefix); return listing; } diff --git a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java index 109137134409..ab5b8151736c 100644 --- a/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java +++ b/gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java @@ -46,7 +46,6 @@ import org.apache.iceberg.io.FileInfo; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; -import org.apache.iceberg.io.PrefixListing; import org.apache.iceberg.io.PrefixListingPage; import org.apache.iceberg.io.StorageCredential; import org.apache.iceberg.io.SupportsStorageCredentials; @@ -307,22 +306,21 @@ public Iterable listPrefix(String prefix) { } @Override - public PrefixListing listPrefix(String prefix, String delimiter) { + public Iterable listPrefixWithDelimiter(String prefix, String delimiter) { if (!"/".equals(delimiter)) { throw new UnsupportedOperationException( String.format("Prefix listing with delimiter '%s' is not supported", delimiter)); } GCSLocation location = new GCSLocation(prefix); - return PrefixListing.of( - () -> - prefixListingPages( - clientForStoragePath(prefix) - .storage() - .list( - location.bucket(), - Storage.BlobListOption.prefix(location.prefix()), - Storage.BlobListOption.currentDirectory()))); + return () -> + prefixListingPages( + clientForStoragePath(prefix) + .storage() + .list( + location.bucket(), + Storage.BlobListOption.prefix(location.prefix()), + Storage.BlobListOption.currentDirectory())); } @Override diff --git a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index a6d4b11a28a2..c17dbad71100 100644 --- a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -668,72 +668,6 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } - @TestTemplate - public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { - if (catalogName.equals("testhadoop")) { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - } else { - sql( - "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", - tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); - } - - sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); - sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); - - Table table = validationCatalog.loadTable(tableIdent); - String dataLocation = table.location() + "/data"; - - sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); - sql("INSERT INTO TABLE p VALUES (1)"); - sql("INSERT INTO TABLE p VALUES (2)"); - - waitUntilAfter(System.currentTimeMillis()); - - Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); - - List output = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output).as("Should find orphan files in the data folder").hasSize(2); - - List output2 = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output2).as("Should be no more orphan files").isEmpty(); - - assertEquals( - "Should have expected rows", - ImmutableList.of(row(1L, "a"), row(2L, "b")), - sql("SELECT * FROM %s ORDER BY id", tableName)); - } - - @TestTemplate - public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - - assertThatThrownBy( - () -> - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, -1)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Prefix listing max seed depth must be non-negative"); - } - @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 9543ebb9be47..b47922820d21 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -131,7 +131,6 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; - private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -223,15 +222,6 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } - public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { - Preconditions.checkArgument( - newMaxSeedDepth >= 0, - "Prefix listing max seed depth must be non-negative: %s", - newMaxSeedDepth); - this.prefixListingMaxSeedDepth = newMaxSeedDepth; - return this; - } - private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -432,36 +422,17 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - List seedPrefixes = Lists.newArrayList(); - if (prefixListingMaxSeedDepth == 0) { - seedPrefixes.add(location); - } else { - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - prefixListingMaxSeedDepth, - MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, - seedPrefixes::add, - matchingFiles::add); - } - - int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); - JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); - ListPrefixes listPrefixes = - new ListPrefixes( - (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); - JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); - - if (matchingFiles.isEmpty()) { - return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); - } + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsPrefixOperations) table.io(), + location, + table.specs(), + predicate, + matchingFiles::add); - JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); + JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -552,31 +523,6 @@ public Iterator call(Iterator dirs) throws Exception { } } - private static class ListPrefixes implements FlatMapFunction, String> { - - private final SupportsPrefixOperations io; - private final long olderThanTimestamp; - private final Map specs; - - ListPrefixes( - SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { - this.io = io; - this.olderThanTimestamp = olderThanTimestamp; - this.specs = specs; - } - - @Override - public Iterator call(Iterator prefixes) { - Predicate predicate = - file -> file.createdAtMillis() < olderThanTimestamp; - return Iterators.concat( - Iterators.transform( - prefixes, - prefix -> - FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); - } - } - private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index 337d5e2fedb9..447f3295c003 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -74,10 +74,6 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of driver-side listing used to discover seed prefixes for parallel prefix - // listing. Only applies when prefix_listing is true. Default is 0 (single seed). - private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = - optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -91,8 +87,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM, - PREFIX_LISTING_MAX_SEED_DEPTH_PARAM + STREAM_RESULTS_PARAM }; private static final StructType OUTPUT_TYPE = @@ -148,7 +143,6 @@ public InternalRow[] call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); - int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -197,7 +191,6 @@ public InternalRow[] call(InternalRow args) { } action.usePrefixListing(prefixListing); - action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); diff --git a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index a29a38750837..a5ac8a7e01ac 100644 --- a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -667,72 +667,6 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } - @TestTemplate - public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { - if (catalogName.equals("testhadoop")) { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - } else { - sql( - "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", - tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); - } - - sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); - sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); - - Table table = validationCatalog.loadTable(tableIdent); - String dataLocation = table.location() + "/data"; - - sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); - sql("INSERT INTO TABLE p VALUES (1)"); - sql("INSERT INTO TABLE p VALUES (2)"); - - waitUntilAfter(System.currentTimeMillis()); - - Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); - - List output = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output).as("Should find orphan files in the data folder").hasSize(2); - - List output2 = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output2).as("Should be no more orphan files").isEmpty(); - - assertEquals( - "Should have expected rows", - ImmutableList.of(row(1L, "a"), row(2L, "b")), - sql("SELECT * FROM %s ORDER BY id", tableName)); - } - - @TestTemplate - public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - - assertThatThrownBy( - () -> - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, -1)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Prefix listing max seed depth must be non-negative"); - } - @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 9543ebb9be47..b47922820d21 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -131,7 +131,6 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; - private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -223,15 +222,6 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } - public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { - Preconditions.checkArgument( - newMaxSeedDepth >= 0, - "Prefix listing max seed depth must be non-negative: %s", - newMaxSeedDepth); - this.prefixListingMaxSeedDepth = newMaxSeedDepth; - return this; - } - private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -432,36 +422,17 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - List seedPrefixes = Lists.newArrayList(); - if (prefixListingMaxSeedDepth == 0) { - seedPrefixes.add(location); - } else { - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - prefixListingMaxSeedDepth, - MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, - seedPrefixes::add, - matchingFiles::add); - } - - int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); - JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); - ListPrefixes listPrefixes = - new ListPrefixes( - (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); - JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); - - if (matchingFiles.isEmpty()) { - return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); - } + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsPrefixOperations) table.io(), + location, + table.specs(), + predicate, + matchingFiles::add); - JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); + JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -552,31 +523,6 @@ public Iterator call(Iterator dirs) throws Exception { } } - private static class ListPrefixes implements FlatMapFunction, String> { - - private final SupportsPrefixOperations io; - private final long olderThanTimestamp; - private final Map specs; - - ListPrefixes( - SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { - this.io = io; - this.olderThanTimestamp = olderThanTimestamp; - this.specs = specs; - } - - @Override - public Iterator call(Iterator prefixes) { - Predicate predicate = - file -> file.createdAtMillis() < olderThanTimestamp; - return Iterators.concat( - Iterators.transform( - prefixes, - prefix -> - FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); - } - } - private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index f9dd6093cca1..89e218c10360 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,10 +80,6 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of driver-side listing used to discover seed prefixes for parallel prefix - // listing. Only applies when prefix_listing is true. Default is 0 (single seed). - private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = - optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -97,8 +93,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM, - PREFIX_LISTING_MAX_SEED_DEPTH_PARAM + STREAM_RESULTS_PARAM }; private static final StructType OUTPUT_TYPE = @@ -154,7 +149,6 @@ public Iterator call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); - int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -203,7 +197,6 @@ public Iterator call(InternalRow args) { } action.usePrefixListing(prefixListing); - action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); diff --git a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index 13764e813350..a5ac8a7e01ac 100644 --- a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -667,90 +667,6 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() sql("DROP TABLE %s", tableName); } - @TestTemplate - public void testRemoveOrphanFilesWithPrefixListingMaxSeedDepth() throws IOException { - if (catalogName.equals("testhadoop")) { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - } else { - sql( - "CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg LOCATION '%s'", - tableName, java.nio.file.Files.createTempDirectory(temp, "junit")); - } - - sql("INSERT INTO TABLE %s VALUES (1, 'a')", tableName); - sql("INSERT INTO TABLE %s VALUES (2, 'b')", tableName); - - Table table = validationCatalog.loadTable(tableIdent); - String dataLocation = table.location() + "/data"; - - // produce orphan files in the data location using parquet - sql("CREATE TABLE p (id bigint) USING parquet LOCATION '%s'", dataLocation); - sql("INSERT INTO TABLE p VALUES (1)"); - sql("INSERT INTO TABLE p VALUES (2)"); - - // wait to ensure files are old enough - waitUntilAfter(System.currentTimeMillis()); - - Timestamp currentTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); - - List output = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output).as("Should find orphan files in the data folder").hasSize(2); - - // subsequent call should find nothing - List output2 = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, currentTimestamp, 2); - assertThat(output2).as("Should be no more orphan files").isEmpty(); - - // seed depth of 0 (default single-seed behavior) should also work - sql("INSERT INTO TABLE p VALUES (3)"); - waitUntilAfter(System.currentTimeMillis()); - Timestamp laterTimestamp = Timestamp.from(Instant.ofEpochMilli(System.currentTimeMillis())); - - List output3 = - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "older_than => TIMESTAMP '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, laterTimestamp, 0); - assertThat(output3).as("Should find the new orphan file with single-seed listing").hasSize(1); - - assertEquals( - "Should have expected rows", - ImmutableList.of(row(1L, "a"), row(2L, "b")), - sql("SELECT * FROM %s ORDER BY id", tableName)); - } - - @TestTemplate - public void testRemoveOrphanFilesWithNegativePrefixListingMaxSeedDepth() { - sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - - assertThatThrownBy( - () -> - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "prefix_listing => true," - + "prefix_listing_max_seed_depth => %d)", - catalogName, tableIdent, -1)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Prefix listing max seed depth must be non-negative"); - } - @TestTemplate public void testRemoveOrphanFilesProcedureWithEqualAuthorities() throws NoSuchTableException, ParseException, IOException { diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java index 9543ebb9be47..b47922820d21 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java @@ -131,7 +131,6 @@ public class DeleteOrphanFilesSparkAction extends BaseSparkAction deleteFunc = null; private ExecutorService deleteExecutorService = null; private boolean usePrefixListing = false; - private int prefixListingMaxSeedDepth = 0; private static final Encoder FILE_URI_ENCODER = Encoders.bean(FileURI.class); DeleteOrphanFilesSparkAction(SparkSession spark, Table table) { @@ -223,15 +222,6 @@ public DeleteOrphanFilesSparkAction usePrefixListing(boolean newUsePrefixListing return this; } - public DeleteOrphanFilesSparkAction prefixListingMaxSeedDepth(int newMaxSeedDepth) { - Preconditions.checkArgument( - newMaxSeedDepth >= 0, - "Prefix listing max seed depth must be non-negative: %s", - newMaxSeedDepth); - this.prefixListingMaxSeedDepth = newMaxSeedDepth; - return this; - } - private Dataset filteredCompareToFileList() { Dataset files = compareToFileList; if (location != null) { @@ -432,36 +422,17 @@ private Dataset listedFileDS() { "Cannot use prefix listing with FileIO %s which does not support prefix operations.", table.io()); - List seedPrefixes = Lists.newArrayList(); - if (prefixListingMaxSeedDepth == 0) { - seedPrefixes.add(location); - } else { - Predicate predicate = - fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; - FileSystemWalker.listDirRecursivelyWithFileIO( - (SupportsPrefixOperations) table.io(), - location, - table.specs(), - predicate, - prefixListingMaxSeedDepth, - MAX_DRIVER_LISTING_DIRECT_SUB_DIRS, - seedPrefixes::add, - matchingFiles::add); - } - - int parallelism = Math.min(Math.max(seedPrefixes.size(), 1), listingParallelism); - JavaRDD seedPrefixRDD = sparkContext().parallelize(seedPrefixes, parallelism); - ListPrefixes listPrefixes = - new ListPrefixes( - (SupportsPrefixOperations) table.io(), olderThanTimestamp, table.specs()); - JavaRDD listPrefixRDD = seedPrefixRDD.mapPartitions(listPrefixes); - - if (matchingFiles.isEmpty()) { - return spark().createDataset(listPrefixRDD.rdd(), Encoders.STRING()); - } + Predicate predicate = + fileInfo -> fileInfo.createdAtMillis() < olderThanTimestamp; + FileSystemWalker.listDirRecursivelyWithFileIO( + (SupportsPrefixOperations) table.io(), + location, + table.specs(), + predicate, + matchingFiles::add); - JavaRDD matchingFilesRDD = sparkContext().parallelize(matchingFiles, 1); - return spark().createDataset(matchingFilesRDD.union(listPrefixRDD).rdd(), Encoders.STRING()); + JavaRDD matchingFileRDD = sparkContext().parallelize(matchingFiles, 1); + return spark().createDataset(matchingFileRDD.rdd(), Encoders.STRING()); } else { Predicate predicate = file -> file.getModificationTime() < olderThanTimestamp; // list at most MAX_DRIVER_LISTING_DEPTH levels and only dirs that have @@ -552,31 +523,6 @@ public Iterator call(Iterator dirs) throws Exception { } } - private static class ListPrefixes implements FlatMapFunction, String> { - - private final SupportsPrefixOperations io; - private final long olderThanTimestamp; - private final Map specs; - - ListPrefixes( - SupportsPrefixOperations io, long olderThanTimestamp, Map specs) { - this.io = io; - this.olderThanTimestamp = olderThanTimestamp; - this.specs = specs; - } - - @Override - public Iterator call(Iterator prefixes) { - Predicate predicate = - file -> file.createdAtMillis() < olderThanTimestamp; - return Iterators.concat( - Iterators.transform( - prefixes, - prefix -> - FileSystemWalker.listDirRecursivelyWithFileIO(io, prefix, specs, predicate))); - } - } - private static class FindOrphanFiles implements MapPartitionsFunction, String> { diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java index f9dd6093cca1..89e218c10360 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RemoveOrphanFilesProcedure.java @@ -80,10 +80,6 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { // Stream results to avoid loading all orphan files in driver memory. Default is false. private static final ProcedureParameter STREAM_RESULTS_PARAM = optionalInParameter("stream_results", DataTypes.BooleanType); - // Depth of driver-side listing used to discover seed prefixes for parallel prefix - // listing. Only applies when prefix_listing is true. Default is 0 (single seed). - private static final ProcedureParameter PREFIX_LISTING_MAX_SEED_DEPTH_PARAM = - optionalInParameter("prefix_listing_max_seed_depth", DataTypes.IntegerType); private static final ProcedureParameter[] PARAMETERS = new ProcedureParameter[] { @@ -97,8 +93,7 @@ public class RemoveOrphanFilesProcedure extends BaseProcedure { EQUAL_AUTHORITIES_PARAM, PREFIX_MISMATCH_MODE_PARAM, PREFIX_LISTING_PARAM, - STREAM_RESULTS_PARAM, - PREFIX_LISTING_MAX_SEED_DEPTH_PARAM + STREAM_RESULTS_PARAM }; private static final StructType OUTPUT_TYPE = @@ -154,7 +149,6 @@ public Iterator call(InternalRow args) { boolean prefixListing = input.asBoolean(PREFIX_LISTING_PARAM, false); boolean streamResults = input.asBoolean(STREAM_RESULTS_PARAM, false); - int prefixListingMaxSeedDepth = input.asInt(PREFIX_LISTING_MAX_SEED_DEPTH_PARAM, 0); return withIcebergTable( tableIdent, @@ -203,7 +197,6 @@ public Iterator call(InternalRow args) { } action.usePrefixListing(prefixListing); - action.prefixListingMaxSeedDepth(prefixListingMaxSeedDepth); if (streamResults) { action.option("stream-results", "true"); diff --git a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java index 80b3b4368397..78e8a0b000a4 100644 --- a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java +++ b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java @@ -1194,46 +1194,6 @@ public void testRemoveOrphanFileActionWithDeleteMode() { DeleteOrphanFiles.PrefixMismatchMode.DELETE); } - @TestTemplate - public void testPrefixListingMaxSeedDepthDiscoversOrphans() throws IOException { - assumeThat(usePrefixListing) - .as("Seed-depth path is only used when prefix listing is enabled") - .isEqualTo(true); - Table table = TABLES.create(SCHEMA, SPEC, properties, tableLocation); - - List records = - Lists.newArrayList(new ThreeColumnRecord(1, "AAAAAAAAAA", "AAAA")); - Dataset df = spark.createDataFrame(records, ThreeColumnRecord.class).coalesce(1); - df.select("c1", "c2", "c3").write().format("iceberg").mode("append").save(tableLocation); - - // orphan files at multiple depths under the table location - df.coalesce(1).write().mode("append").parquet(tableLocation + "/data/invalid"); - df.coalesce(1).write().mode("append").parquet(tableLocation + "/data/nested/invalid"); - - waitUntilAfter(System.currentTimeMillis()); - - DeleteOrphanFiles.Result result = - SparkActions.get() - .deleteOrphanFiles(table) - .usePrefixListing(true) - .prefixListingMaxSeedDepth(2) - .olderThan(System.currentTimeMillis()) - .execute(); - - assertThat(result.orphanFileLocations()) - .as("Seed-depth prefix listing should still discover orphan files at deeper levels") - .isNotEmpty(); - } - - @TestTemplate - public void testPrefixListingMaxSeedDepthRejectsNegative() { - Table table = TABLES.create(SCHEMA, PartitionSpec.unpartitioned(), properties, tableLocation); - assertThatThrownBy( - () -> SparkActions.get().deleteOrphanFiles(table).prefixListingMaxSeedDepth(-1)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Prefix listing max seed depth must be non-negative"); - } - @TestTemplate public void testDefaultToHadoopListing() { assumeThat(usePrefixListing) From a5259c137c78e8165d75754a379c405932e90a50 Mon Sep 17 00:00:00 2001 From: Thomas Powell Date: Tue, 6 Oct 2026 22:16:32 +0100 Subject: [PATCH 10/10] fix checkstyle --- .../main/java/org/apache/iceberg/util/FileSystemWalker.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java index 4055bd87f296..59eafe4509a6 100644 --- a/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java +++ b/core/src/main/java/org/apache/iceberg/util/FileSystemWalker.java @@ -184,10 +184,6 @@ private static void listDirRecursivelyWithFileIO( } } - if (maxDirectSubDirsExceeded) { - return; - } - for (String subDir : subDirs) { listDirRecursivelyWithFileIO( io,