Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/24712 )
Change subject: IMPALA-13314: Cache HadoopCatalog instances and use holder pattern for singletons ...................................................................... Patch Set 3: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/24712/1/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java: http://gerrit.cloudera.org:8080/#/c/24712/1/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java@56 PS1, Line 56: private static final Cache<String, IcebergHadoopCatalog> catalogCache_ = : CacheBuilder.newBuilder() : .maximumSize(MAX_CACHE_SIZE) : .removalListener(notification -> { : IcebergHadoopCatalog evicted = (IcebergHadoopCatalog) notification.getValue(); : try { : evicted.hadoopCatalog_.close(); : } catch (IOException e) { : LOG.warn("Failed to close evicted HadoopCatalog for location: {}", : notification.getKey(), e); : } : }) : .build(); : > Thanks, a great observation! I would go with something like a soft ref approach and defer the removal to the cases when the JVM has memory pressure, this makes the eviction automatic and not limited by a parameter. http://gerrit.cloudera.org:8080/#/c/24712/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java File fe/src/main/java/org/apache/impala/util/IcebergUtil.java: http://gerrit.cloudera.org:8080/#/c/24712/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@187 PS1, Line 187: }; > As we cover every possible case in the enum at the moment, the default is r I'm good with this approach, in this way, "throws ImpalaRuntimeException" should be removed -- To view, visit http://gerrit.cloudera.org:8080/24712 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb3a6c8e4f1d2a9e7c5b0f8d3e6a4c2b1d0e9f7a Gerrit-Change-Number: 24712 Gerrit-PatchSet: 3 Gerrit-Owner: Nandor Kollar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Nandor Kollar <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Comment-Date: Tue, 25 Aug 2026 11:24:01 +0000 Gerrit-HasComments: Yes
