pvary commented on code in PR #5378:
URL: https://github.com/apache/iceberg/pull/5378#discussion_r933632062
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/CachedClientPool.java:
##########
@@ -56,9 +57,16 @@ public class CachedClientPool implements
ClientPool<IMetaStoreClient, TException
init();
}
+ @VisibleForTesting
+ static String cacheKey(Configuration conf) {
+ return String.format(
+ "%s:%s",
+ conf.get(HiveConf.ConfVars.METASTOREURIS.varname, ""),
conf.get(CATALOG_DEFAULT, ""));
Review Comment:
If I understand correctly Spark sends every configuration value to the HMS
Client which starts with `spark.sql.catalog.catalog1.hadoop.` in this case.
Choosing `metastore.catalog.default` as the only thing used in the cacheKey
seems like an ad-hoc decision to me. Some interesting configs:
- CLIENT_SOCKET_TIMEOUT
- HMS_HANDLER_ATTEMPTS
- HMS_HANDLER_INTERVAL
Different users might require different values here...
Would it be "overengineering" to have a static list of config keys to use in
the `cacheKey`, and use a hash, or a join of the values for the cache key?
--
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]
