Nandor Kollar 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: (1 comment) 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(); : > With closing the catalog, there's a race condition window where the evictio Thanks, a great observation! Two possible options came to my mind: - Ignore the problem, by deleting the removalListener, and not close the catalogs. This might cause resource leaks, which we already have too. - Reference counting. Not sure that it worth the effort, as it doesn't sound trivial. What do you think? Do you happen to have any alternative recommendation? -- 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: Wed, 19 Aug 2026 14:16:45 +0000 Gerrit-HasComments: Yes
