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 16: (6 comments) PS 16 fixed Zoltan's comments and also did a rebase, which included the ehcache removal (IMPALA-15114) http://gerrit.cloudera.org:8080/#/c/24049/15//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24049/15//COMMIT_MSG@19 PS15, Line 19: load_iceberg_files_in_coordinator=true does not : work with legacy catalog mode > We could check incompatible flag combinations at startup. The flags are experimental, I would put effort in this once there will be flags usable in production. Preferably it would be enough to set this in coordinator or catalogd, just didn't decide which one. 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@1364 PS9, Line 1364: etaProvider.CachedIcebergFiles>() { > As Impala bumped to Java 17, I plan to remove Ehcache. Created IMPALA-15114 IMPALA-15114 was merged and ehcache was removed. http://gerrit.cloudera.org:8080/#/c/24049/15/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/15/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@1354 PS15, Line 1354: org.apache.iceberg.Table apiTable = : loadIcebergApiTable(tableRef, tableParams, tableRef.msTable_); : : String itemStr = "content file store for " + tableRef.dbName_ + "." : + tableRef.tableName_; : // TODO: use snapshot id for key to make usable for REST catalog : IcebergContentFileStoreCacheKey cacheKey = : new IcebergContentFileStoreCacheKey(tableRef); : MetaProvider.CachedIcebergFiles result = : loadWithCaching(itemStr, TABLE_METADATA_CACHE_CATEGORY, cacheKey, : new Callable<MetaProvider.CachedIcebergFiles>() { : @Override : public MetaProvider.CachedIcebergFiles call() throws Exception { : // Try to lookup the last stored IcebergContentFileStore to help with : // incremental loading. A weak reference is used to allow it to be GCd : // if it gets evicted from cache. The key doesn't include catalog version, : // so there is always only o > This is not needed when the files are in the cache. Would it be worth addin Good idea, done. I was worried about this because it is not legal to call cache functions in the cache's functions like compute(), but the logic below does not run within compute() http://gerrit.cloudera.org:8080/#/c/24049/15/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/24049/15/fe/src/main/java/org/apache/impala/catalog/local/IcebergMetaProvider.java@599 PS15, Line 599: > nit: missing space Done http://gerrit.cloudera.org:8080/#/c/24049/15/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/15/fe/src/main/java/org/apache/impala/service/BackendConfig.java@660 PS15, Line 660: InCoordinator > nit: InCoordinator Done http://gerrit.cloudera.org:8080/#/c/24049/15/tests/custom_cluster/test_iceberg_load_on_coordinator.py File tests/custom_cluster/test_iceberg_load_on_coordinator.py: http://gerrit.cloudera.org:8080/#/c/24049/15/tests/custom_cluster/test_iceberg_load_on_coordinator.py@32 PS15, Line 32: # TODO: think through what tests to run in with special config. > How much time does it take to run these tests? Measured the tests. These can be run in parallel as all tests need the same custom cluster. Maybe Impala's test suites don't do that for custom cluster tests? time impala-py.test tests/custom_cluster/test_iceberg_load_on_coordinator.py real 4m59.816s time impala-py.test -n6 tests/custom_cluster/test_iceberg_load_on_coordinator.py real 1m40.950s -- 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: 16 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: Tue, 30 Jun 2026 12:23:46 +0000 Gerrit-HasComments: Yes
