szehon-ho commented on code in PR #6698:
URL: https://github.com/apache/iceberg/pull/6698#discussion_r1092703260
##########
core/src/main/java/org/apache/iceberg/CatalogProperties.java:
##########
@@ -119,6 +119,8 @@ private CatalogProperties() {}
"client.pool.cache.eviction-interval-ms";
public static final long CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS_DEFAULT =
TimeUnit.MINUTES.toMillis(5);
+ /** Name of the custom {@link ClientPool} implementation class. */
+ public static final String CLIENT_POOL_IMPL = "client-pool-impl";
Review Comment:
I think the context is from https://github.com/apache/iceberg/pull/6175
(more issues scattered). The general problem being the CachedClientPool that's
always on in Iceberg Hive, which is a global cache that doesnt work in all
environments, because the current key (metastoreUri) collides too much, leading
to the wrong client used for a lot of use cases.
It seems simpler to me to just make cache pluggable, but I am not sure if
there's a better solution I'm not seeing to make it non-dynamic? If we dont
want user to use a client-pool other than CachedClientPool, at least we need to
have a pluggable conf => key generator? Or maybe just allow them to use the
toggle between CachedClientPool and the raw ClientPool as an option (as a way
to turn off the global cache)? cc @pvary @flyrain as well if any thoughts
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
