Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22014 )
Change subject: IMPALA-13154: Update metrics when loading an HDFS table ...................................................................... Patch Set 18: (2 comments) I think the patch is in a pretty good shape. Just have a minor comment. http://gerrit.cloudera.org:8080/#/c/22014/18/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/22014/18/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1755 PS18, Line 1755: if (thriftPartition.isSetFile_desc()) { : setFileDescriptors(fdsFromThrift(thriftPartition.getFile_desc())); : } : if (thriftPartition.isSetInsert_file_desc()) { : setInsertFileDescriptors(fdsFromThrift(thriftPartition.getInsert_file_desc())); : } : if (thriftPartition.isSetDelete_file_desc()) { : setDeleteFileDescriptors(fdsFromThrift(thriftPartition.getDelete_file_desc())); : } As set*FileDescriptors() are called here, I think we can generate FileMetadataStats and set it here so they are always consistent. We already did so in ParallelFileMetadataLoader#load(). This simplifies our checks in HdfsTable#addPartitionNoThrow() and HdfsTable#dropPartition() about whether the partition is loaded from thrift. http://gerrit.cloudera.org:8080/#/c/22014/18/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/22014/18/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1044 PS18, Line 1044: fileMetadataStats_.numFiles += partition.getNumFileDescriptors(); It's strange that 'numBlocks' is not updated here and even in the original code. Maybe it's due to we haven't used it at all. -- To view, visit http://gerrit.cloudera.org:8080/22014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e2eb503b0f61b1e6403058bc5dc78d721e7e940 Gerrit-Change-Number: 22014 Gerrit-PatchSet: 18 Gerrit-Owner: Xuebin Su <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Xuebin Su <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 12 Dec 2024 07:30:35 +0000 Gerrit-HasComments: Yes
