Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23906 )
Change subject: IMPALA-13122: Add detailed file metadata statistics to table loading logs ...................................................................... Patch Set 9: (7 comments) authorization/test_ranger.py::TestRangerIndependent::test_allow_metadata_update_local_catalog failed which I dont think is related to the changes. http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@252 PS7, Line 252: LogString(String > nit: import java.time.ZoneId Done http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@256 PS7, Line 256: ngBuilder statsLog = new StringBuilder() > Let's skip this for non-partitioned tables. It's confusing to see logs like Done http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@257 PS7, Line 257: metadata for ").append(tableName); > nit: this class is already imported on L49 so we can use the class name dir Done http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@275 PS7, Line 275: axFileBytes)); > nit: import java.time.Instant to use Instant directly. Done http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java File fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java@242 PS7, Line 242: > I think we can declare minAccessTime_ and maxAccessTime_ as local variables Done http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java File fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java: http://gerrit.cloudera.org:8080/#/c/23906/7/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java@134 PS7, Line 134: rtTrue(stats.getNumUniqueHosts() <= stats > Why do we need this OR statement? Is there are case that number of hosts > No, hosts are derived from the host:disk pairs. Done http://gerrit.cloudera.org:8080/#/c/23906/7/tests/custom_cluster/test_file_metadata_stats.py File tests/custom_cluster/test_file_metadata_stats.py: http://gerrit.cloudera.org:8080/#/c/23906/7/tests/custom_cluster/test_file_metadata_stats.py@29 PS7, Line 29: catalog > nit: "catalogd", we usually use the term "catalog" to mean all the metadata Done -- To view, visit http://gerrit.cloudera.org:8080/23906 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f4592f173c047e5064058402f83be6d1f5c9a79 Gerrit-Change-Number: 23906 Gerrit-PatchSet: 9 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Sat, 28 Feb 2026 02:40:22 +0000 Gerrit-HasComments: Yes
