Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24049 )
Change subject: WIP IMPALA-14805: LocalIcebergTable loads files in coordinator ...................................................................... Patch Set 15: (14 comments) http://gerrit.cloudera.org:8080/#/c/24049/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24049/7//COMMIT_MSG@24 PS7, Line 24: This would b > nit: weak reference Done http://gerrit.cloudera.org:8080/#/c/24049/7//COMMIT_MSG@49 PS7, Line 49: tor's defaul > nit: inconsistent Done http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java File fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java: http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/IcebergContentFileStore.java@162 PS7, Line 162: * @param maxFiles Maximum files to include : * @return Thrift map for > At some places we have BEGIN / END sections to make it more explicit. See e Done http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1257 PS7, Line 1257: ebergContentFileStoreCacheKey cacheKey = > nit: fits earlier line Done http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1263 PS7, Line 1263: > nit: to Done http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1265 PS7, Line 1265: ddr > Nit: one Done http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1250 PS9, Line 1250: if > nit: need a space Done http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1360 PS9, Line 1360: to be GCd > nit: "to be GCed" Done http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1362 PS9, Line 1362: one > nit: "one" Done http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1364 PS9, Line 1364: The weigher does not go through weak references > Is it true for Ehcache SizeOf? I don't see code handling this in https://gi As Impala bumped to Java 17, I plan to remove Ehcache. Created IMPALA-15114 http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@138 PS7, Line 138: chema if it's ext > Is it necessary to have this? The internal FS table should be minimal since As removed in later PS. http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@98 PS7, Line 98: > It is only written, never read. Was removed in later patch. http://gerrit.cloudera.org:8080/#/c/24049/7/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@158 PS7, Line 158: tableParams_ = tableParams; > LocalTable.load() invokes t.loadColumnStats() -> metaProvider.loadTableColu I kept the warmup for now - the risk of updating the table while the file list is loaded and making the old columns stats unreachable is still there. http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/24049/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java@640 PS9, Line 640: isOtelTraceEnabled() { > nit: "getLoadIcebergFilesInCoordinator" Done -- To view, visit http://gerrit.cloudera.org:8080/24049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6732af76a2e040fa57e39260302951466037b934 Gerrit-Change-Number: 24049 Gerrit-PatchSet: 15 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 22 Jun 2026 17:36:32 +0000 Gerrit-HasComments: Yes
