You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/07/25 21:48:17 UTC
[GitHub] [hudi] vinothchandar commented on a diff in pull request #5829: [HUDI-4221] Fixing getAllPartitionPaths perf hit w/ FileSystemBackedMetadata
vinothchandar commented on code in PR #5829:
URL: https://github.com/apache/hudi/pull/5829#discussion_r929286057
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java:
##########
@@ -68,41 +68,46 @@ public FileStatus[] getAllFilesInPartition(Path partitionPath) throws IOExceptio
@Override
public List<String> getAllPartitionPaths() throws IOException {
- FileSystem fs = new Path(datasetBasePath).getFileSystem(hadoopConf.get());
+ Path basePath = new Path(datasetBasePath);
+ FileSystem fs = basePath.getFileSystem(hadoopConf.get());
if (assumeDatePartitioning) {
return FSUtils.getAllPartitionFoldersThreeLevelsDown(fs, datasetBasePath);
}
List<Path> pathsToList = new CopyOnWriteArrayList<>();
- pathsToList.add(new Path(datasetBasePath));
+ pathsToList.add(basePath);
List<String> partitionPaths = new CopyOnWriteArrayList<>();
while (!pathsToList.isEmpty()) {
// TODO: Get the parallelism from HoodieWriteConfig
int listingParallelism = Math.min(DEFAULT_LISTING_PARALLELISM, pathsToList.size());
// List all directories in parallel
- List<FileStatus[]> dirToFileListing = engineContext.map(pathsToList, path -> {
+ List<Pair<Path, FileStatus[]>> dirToFileListing = engineContext.map(pathsToList, path -> {
FileSystem fileSystem = path.getFileSystem(hadoopConf.get());
- return fileSystem.listStatus(path);
+ return Pair.of(path, fileSystem.listStatus(path));
}, listingParallelism);
pathsToList.clear();
// if current dictionary contains PartitionMetadata, add it to result
// if current dictionary does not contain PartitionMetadata, add it to queue
- dirToFileListing.stream().flatMap(Arrays::stream).parallel()
- .forEach(fileStatus -> {
- if (fileStatus.isDirectory()) {
- if (HoodiePartitionMetadata.hasPartitionMetadata(fs, fileStatus.getPath())) {
- partitionPaths.add(FSUtils.getRelativePartitionPath(new Path(datasetBasePath), fileStatus.getPath()));
- } else if (!fileStatus.getPath().getName().equals(HoodieTableMetaClient.METAFOLDER_NAME)) {
- pathsToList.add(fileStatus.getPath());
- }
- } else if (fileStatus.getPath().getName().startsWith(HoodiePartitionMetadata.HOODIE_PARTITION_METAFILE_PREFIX)) {
- String partitionName = FSUtils.getRelativePartitionPath(new Path(datasetBasePath), fileStatus.getPath().getParent());
- partitionPaths.add(partitionName);
- }
- });
+ dirToFileListing.forEach(p -> {
Review Comment:
don't we have an utilty for this in FSUtils.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java:
##########
@@ -82,27 +82,31 @@ public List<String> getAllPartitionPaths() throws IOException {
int listingParallelism = Math.min(DEFAULT_LISTING_PARALLELISM, pathsToList.size());
// List all directories in parallel
- List<FileStatus[]> dirToFileListing = engineContext.map(pathsToList, path -> {
+ List<Pair<Path, FileStatus[]>> dirToFileListing = engineContext.map(pathsToList, path -> {
FileSystem fileSystem = path.getFileSystem(hadoopConf.get());
- return fileSystem.listStatus(path);
+ return Pair.of(path, fileSystem.listStatus(path));
}, listingParallelism);
pathsToList.clear();
// if current dictionary contains PartitionMetadata, add it to result
// if current dictionary does not contain PartitionMetadata, add it to queue
- dirToFileListing.stream().flatMap(Arrays::stream).parallel()
- .forEach(fileStatus -> {
- if (fileStatus.isDirectory()) {
- if (HoodiePartitionMetadata.hasPartitionMetadata(fs, fileStatus.getPath())) {
- partitionPaths.add(FSUtils.getRelativePartitionPath(new Path(datasetBasePath), fileStatus.getPath()));
- } else if (!fileStatus.getPath().getName().equals(HoodieTableMetaClient.METAFOLDER_NAME)) {
- pathsToList.add(fileStatus.getPath());
- }
- } else if (fileStatus.getPath().getName().startsWith(HoodiePartitionMetadata.HOODIE_PARTITION_METAFILE_PREFIX)) {
- String partitionName = FSUtils.getRelativePartitionPath(new Path(datasetBasePath), fileStatus.getPath().getParent());
- partitionPaths.add(partitionName);
- }
- });
+ dirToFileListing.forEach(p -> {
+ Option<FileStatus> partitionMetaFile = Option.fromJavaOptional(Arrays.stream(p.getRight()).parallel()
+ .filter(fileStatus -> fileStatus.getPath().getName().equals(HoodiePartitionMetadata.HOODIE_PARTITION_METAFILE_PREFIX))
+ .findFirst());
+
+ if (partitionMetaFile.isPresent()) {
+ // Is a partition.
+ String partitionName = FSUtils.getRelativePartitionPath(new Path(datasetBasePath), p.getLeft());
+ partitionPaths.add(partitionName);
+ } else {
+ // Add sub-dirs to the queue
+ pathsToList.addAll(Arrays.stream(p.getRight())
+ .filter(fileStatus -> fileStatus.isDirectory() && !fileStatus.getPath().getName().equals(HoodieTableMetaClient.METAFOLDER_NAME))
Review Comment:
is there a follow up JIRA?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java:
##########
@@ -82,27 +83,35 @@ public List<String> getAllPartitionPaths() throws IOException {
int listingParallelism = Math.min(DEFAULT_LISTING_PARALLELISM, pathsToList.size());
// List all directories in parallel
- List<FileStatus[]> dirToFileListing = engineContext.map(pathsToList, path -> {
+ List<Pair<Path, FileStatus[]>> dirToFileListing = engineContext.map(pathsToList, path -> {
FileSystem fileSystem = path.getFileSystem(hadoopConf.get());
- return fileSystem.listStatus(path);
+ return Pair.of(path, fileSystem.listStatus(path));
}, listingParallelism);
pathsToList.clear();
// if current dictionary contains PartitionMetadata, add it to result
// if current dictionary does not contain PartitionMetadata, add it to queue
- dirToFileListing.stream().flatMap(Arrays::stream).parallel()
Review Comment:
+1 . I think if we simply do the processing we need to do within the single engineContext.map(..) call, we should be able to solve the original problem in #4643.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org