kumarpritam863 commented on PR #18894: URL: https://github.com/apache/iceberg/pull/18894#issuecomment-6890922284
On clientBuilder/clientPoolBuilder, I checked how they behave today. The Flink CatalogLoaders go through CatalogUtil.loadCatalog (no-arg constructor, then initialize), so the builders are never set and each catalog owns its client or pool. A catalog also closes whatever its builder returns. JdbcCatalog.close() closed an injected shared pool, and a second catalog on that pool then failed with "Cannot get a client from a closed pool". RESTSessionCatalog.initialize calls the builder twice and closes the first result immediately. Sharing through those hooks would need the builder to return something whose close() is safe, or a contract change in the catalogs. The rule already used in the repo is that whoever loads a catalog closes it: TableLoader.CatalogTableLoader.close() (called from IcebergFilesCommitter.close()) and Kafka Connect's IcebergSinkTask. This PR applies that to the dynamic sink operators. I'm happy to follow whatever policy you, @nastra , @mxm and @Guosmilesmile settle on. If the decision is not to close catalogs, I'll close this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
