Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce Hive file system listing #18179

Merged
merged 2 commits into from
Jul 14, 2023
Merged

Reduce Hive file system listing #18179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0ef639
Reduce Hive file system listing
alexjo2144 Jul 7, 2023
f1bee05
empty
alexjo2144 Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsFileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.filesystem.hdfs;

import com.google.common.collect.ImmutableMap;
import io.airlift.stats.TimeStat;
import io.trino.filesystem.FileIterator;
import io.trino.filesystem.Location;
Expand Down Expand Up @@ -48,6 +49,13 @@
class HdfsFileSystem
implements TrinoFileSystem
{
private static final Map<String, Boolean> KNOWN_HIERARCHICAL_FILESYSTEMS = ImmutableMap.<String, Boolean>builder()
.put("s3", false)
.put("s3a", false)
.put("s3n", false)
.put("hdfs", true)
.buildOrThrow();

private final HdfsEnvironment environment;
private final HdfsContext context;
private final TrinoHdfsFileSystemStats stats;
Expand Down Expand Up @@ -224,6 +232,11 @@ public Optional<Boolean> directoryExists(Location location)

private boolean hierarchical(FileSystem fileSystem, Location rootLocation)
{
Boolean knownResult = KNOWN_HIERARCHICAL_FILESYSTEMS.get(fileSystem.getScheme());
if (knownResult != null) {
return knownResult;
}

Boolean cachedResult = hierarchicalFileSystemCache.get(fileSystem);
if (cachedResult != null) {
return cachedResult;
Expand Down
19 changes: 10 additions & 9 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/BackgroundHiveSplitLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,12 +563,12 @@ private ListenableFuture<Void> loadPartition(HivePartitionMetadata partition)

private List<TrinoFileStatus> listBucketFiles(TrinoFileSystem fs, Location location, String partitionName)
{
if (!ignoreAbsentPartitions) {
checkPartitionLocationExists(fs, location);
}

try {
return ImmutableList.copyOf(new HiveFileIterator(table, location, fs, directoryLister, hdfsNamenodeStats, FAIL));
HiveFileIterator fileIterator = new HiveFileIterator(table, location, fs, directoryLister, hdfsNamenodeStats, FAIL);
Copy link
Member

@losipiuk losipiuk Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will iterator creation report error if location does not exists? If so will this error be as nice as one generated by checkPartitionLocationExists?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will iterator creation report error if location does not exists?

It will not, that's why the check is there.

if (!fileIterator.hasNext() && !ignoreAbsentPartitions) {
checkPartitionLocationExists(fs, location);
}
return ImmutableList.copyOf(fileIterator);
}
catch (HiveFileIterator.NestedDirectoryNotAllowedException e) {
// Fail here to be on the safe side. This seems to be the same as what Hive does
Expand Down Expand Up @@ -651,9 +651,11 @@ Optional<Iterator<InternalHiveSplit>> buildManifestFileIterator(
TrinoFileSystem trinoFileSystem = fileSystemFactory.create(session);
Location location = Location.of(parent.toString());

checkPartitionLocationExists(trinoFileSystem, location);
Map<Path, TrinoFileStatus> fileStatuses = new HashMap<>();
HiveFileIterator fileStatusIterator = new HiveFileIterator(table, location, trinoFileSystem, directoryLister, hdfsNamenodeStats, IGNORED);
if (!fileStatusIterator.hasNext()) {
checkPartitionLocationExists(trinoFileSystem, location);
}
fileStatusIterator.forEachRemaining(status -> fileStatuses.put(getPathWithoutSchemeAndAuthority(new Path(status.getPath())), status));

List<TrinoFileStatus> locatedFileStatuses = new ArrayList<>();
Expand Down Expand Up @@ -814,11 +816,10 @@ private static boolean shouldUseFileSplitsFromInputFormat(InputFormat<?, ?> inpu

private Iterator<InternalHiveSplit> createInternalHiveSplitIterator(TrinoFileSystem fileSystem, Location location, InternalHiveSplitFactory splitFactory, boolean splittable, Optional<AcidInfo> acidInfo)
{
if (!ignoreAbsentPartitions) {
Iterator<TrinoFileStatus> iterator = new HiveFileIterator(table, location, fileSystem, directoryLister, hdfsNamenodeStats, recursiveDirWalkerEnabled ? RECURSE : IGNORED);
if (!iterator.hasNext() && !ignoreAbsentPartitions) {
checkPartitionLocationExists(fileSystem, location);
}

Iterator<TrinoFileStatus> iterator = new HiveFileIterator(table, location, fileSystem, directoryLister, hdfsNamenodeStats, recursiveDirWalkerEnabled ? RECURSE : IGNORED);
return createInternalHiveSplitIterator(splitFactory, splittable, acidInfo, Streams.stream(iterator));
}

Expand Down