Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22872 )
Change subject: IMPALA-12162: Use thread pool to collect checksums ...................................................................... Patch Set 15: (3 comments) http://gerrit.cloudera.org:8080/#/c/22872/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java File fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/22872/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java@126 PS12, Line 126: > I don't think we need. partBuilders are independent to each other. Ack http://gerrit.cloudera.org:8080/#/c/22872/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java@128 PS12, Line 128: } : } : : private static void updatePartBuilders(FileMetadataLoader loader, : List<HdfsPartition.Builder> builders) { : // Store the loaded FDs into the partitions. : for (HdfsPartition.Builder partBuilder : builders) { : // Checks if we can reuse the old file descriptors. Partition builders in the : // list may have different file descriptors. We need to verify them one by one. : if ((!loader.hasFilesChangedCompareTo(partBuilder.getFileDescriptors()))) { : LOG.trace("Detected files unchanged on partition {}", : partBuilder.getPartitionName()); : continue; : } : partBuilder.clearFileDescriptors(); : List<FileDescriptor> deleteDescriptors = loader.getLoadedDeleteDeltaFds(); : if (deleteDescriptors != null && !deleteDescriptors.isEmpty()) { : partBuilder.setInsertFileDescriptors(loader.getLoadedInsertDeltaFds()); : p > nit: it'd be more clean if we wrap these into a method, e.g. Done http://gerrit.cloudera.org:8080/#/c/22872/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java@193 PS12, Line 193: try { > This seems something new added by this patch. Any reason for it? Some slightly cleaner handling for InterruptedException, but we don't leverage it right now. Something slightly different is done in https://gerrit.cloudera.org/c/22738/2/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java. Removed. -- To view, visit http://gerrit.cloudera.org:8080/22872 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I314621104e4757620c0a90d41dd6875bf8855b51 Gerrit-Change-Number: 22872 Gerrit-PatchSet: 15 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Thu, 22 May 2025 19:07:39 +0000 Gerrit-HasComments: Yes
