suryaprasanna commented on code in PR #18417:
URL: https://github.com/apache/hudi/pull/18417#discussion_r3060308133
##########
hudi-common/src/main/java/org/apache/hudi/BaseHoodieTableFileIndex.java:
##########
@@ -385,7 +394,13 @@ protected List<PartitionPath>
listPartitionPaths(List<String> relativePartitionP
HoodieTimeline timelineToQuery = findInstantsInRange();
matchedPartitionPaths =
TimelineUtils.getWrittenPartitions(timelineToQuery);
} else {
Review Comment:
This was missed out in the previous draft, fixed it in the latest iteration.
##########
hudi-common/src/main/java/org/apache/hudi/BaseHoodieTableFileIndex.java:
##########
@@ -357,18 +358,26 @@ private Map<PartitionPath, List<FileSlice>>
filterFiles(List<PartitionPath> part
.orElseGet(() ->
finalFileSystemView.getLatestFileSlices(partitionPath.path))
.collect(Collectors.toList())
));
+ } finally {
+ log.debug("On {} with query instant as {}, it took {}ms to filter {}
files into file slices across {} partitions",
+ metaClient.getTableConfig().getTableName(),
queryInstant.orElse("N/A"), timer.endTimer(),
+ allFiles.size(), partitions.size());
}
}
protected List<PartitionPath> listPartitionPaths(List<String>
relativePartitionPaths,
Types.RecordType
partitionFields,
Expression
partitionColumnPredicates) {
List<String> matchedPartitionPaths;
+ HoodieTimer timer = HoodieTimer.start();
try {
matchedPartitionPaths =
tableMetadata.getPartitionPathWithPathPrefixUsingFilterExpression(relativePartitionPaths,
partitionFields, partitionColumnPredicates);
} catch (IOException e) {
throw new HoodieIOException("Error fetching partition paths", e);
+ } finally {
+ log.debug("On {}, it took {} ms to list partition paths with {}
relativePartitionPaths and partition predicates",
Review Comment:
Made the change.
##########
hudi-common/src/main/java/org/apache/hudi/BaseHoodieTableFileIndex.java:
##########
@@ -384,13 +393,19 @@ protected List<PartitionPath>
listPartitionPaths(List<String> relativePartitionP
HoodieTimeline timelineToQuery = findInstantsInRange();
matchedPartitionPaths =
TimelineUtils.getWrittenPartitions(timelineToQuery);
} else {
- matchedPartitionPaths =
tableMetadata.getPartitionPathWithPathPrefixes(relativePartitionPaths);
+ HoodieTimer timer = HoodieTimer.start();
Review Comment:
Those timers belong to different methods, they are not related.
##########
hudi-common/src/main/java/org/apache/hudi/BaseHoodieTableFileIndex.java:
##########
@@ -476,6 +491,12 @@ private List<StoragePathInfo>
listPartitionPathFiles(List<PartitionPath> partiti
Set<StoragePath> missingPartitionPaths =
CollectionUtils.diffSet(partitionPaths, cachedPartitionPaths.keySet());
+ if (missingPartitionPaths.isEmpty()) {
+ return cachedPartitionPaths.values().stream()
+ .flatMap(Collection::stream)
+ .collect(Collectors.toList());
+ }
+
Review Comment:
Done.
##########
hudi-common/src/main/java/org/apache/hudi/BaseHoodieTableFileIndex.java:
##########
@@ -502,7 +527,10 @@ private List<StoragePathInfo>
listPartitionPathFiles(List<PartitionPath> partiti
return result;
} catch (IOException e) {
- throw new HoodieIOException("Failed to list partition paths", e);
+ throw new HoodieIOException("On " +
metaClient.getTableConfig().getTableName() + " Failed to list partition paths",
e);
+ } finally {
+ log.debug("On {}, it took {} ms to fetch files for {} uncached
partitions via getAllFilesInPartitions",
+ metaClient.getTableConfig().getTableName(), timer.endTimer(),
missingPartitionPaths.size());
Review Comment:
Good catch, better to move the endTimer outside debug statement, otherise
logger can potentially ignore it and the timer might be alive.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
