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 turn off 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: [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]
