Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/18160 )
Change subject: IMPALA-11076: Re-use FDs loaded by HdfsTable during IcebergTable load ...................................................................... Patch Set 1: (2 comments) Hi Zoltank, thank you for the review, the CR has been updated. http://gerrit.cloudera.org:8080/#/c/18160/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/18160/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@472 PS1, Line 472: /** : * Get FileDescriptor by data file location : */ : public static HdfsPartition.FileDescriptor getFileDescriptor(Path fileLoc, : Path tableLoc, ListMap<TNetworkAddress> hostIndex) throws IOException { : FileSystem fs = FileSystemUtil.getFileSystemForPath(tableLoc); : FileStatus fileStatus = fs.getFileStatus(fileLoc); : return getFileDescriptor(fs, tableLoc, fileStatus, hostIndex); : } : : private static HdfsPartition.FileDescriptor getFileDescriptor(FileSystem fs, : Path tableLoc, FileStatus fileStatus, ListMap<TNetworkAddress> hostIndex) : throws IOException { : Reference<Long> numUnknownDiskIds = new Reference<Long>(Long.valueOf(0)); : String relPath = FileSystemUtil.relativizePath(fileStatus.getPath(), tableLoc); : : if (!FileSystemUtil.supportsStorageIds(fs)) { : return HdfsPartition.FileDescriptor.createWithNoBlocks(fileStatus, relPath); : } : : BlockLocation[] locations; : if (fileStatus instanceof LocatedFileStatus) { : locations = ((LocatedFileStatus)fileStatus).getBlockLocations(); : } else { : locations = fs.getFileBlockLocations(fileStatus, 0, fileStatus.getLen()); : } : : return HdfsPartition.FileDescriptor.create(fileStatus, relPath, locations, : hostIndex, HdfsShim.isErasureCoded(fileStatus), numUnknownDiskIds); : } > Becomes unused? Or maybe we can keep it as a fallback? E.g. generate a warn These are used by IcebergScanNode.getFileDescriptorByIcebergPredicates() as well. Maybe I need to increase the scope. http://gerrit.cloudera.org:8080/#/c/18160/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@516 PS1, Line 516: fileDescMap > We should only put entries to the fileDescMap if they are actually in the I Thanks for the pointers, this part has been updated. -- To view, visit http://gerrit.cloudera.org:8080/18160 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id49daec60237dd6c8cce92c00a09fd9a6e6479b4 Gerrit-Change-Number: 18160 Gerrit-PatchSet: 1 Gerrit-Owner: Tamas Mate <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 26 Jan 2022 16:31:49 +0000 Gerrit-HasComments: Yes
