marton-bod commented on a change in pull request #2325:
URL: https://github.com/apache/iceberg/pull/2325#discussion_r593308684



##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
##########
@@ -547,4 +541,42 @@ public void setConf(Configuration conf) {
   public Configuration getConf() {
     return conf;
   }
+
+  @VisibleForTesting
+  HiveClientPool clientPool() {
+    synchronized (CLIENT_POOL_CACHE) {
+      String metastoreUri = conf.get(HiveConf.ConfVars.METASTOREURIS.varname, 
"");
+      Pair<HiveClientPool, Long> cacheEntry = 
CLIENT_POOL_CACHE.getIfPresent(metastoreUri);
+      HiveClientPool clientPool = cacheEntry == null ? new 
HiveClientPool(clientPoolSize, conf) : cacheEntry.first();

Review comment:
       Can we restrict the scope of the synchronised block to contain only the 
cache population part? that way we could decrease the contention on the more 
frequent cache lookup operation. If `cacheEntry` is null, we enter the block, 
and re-check if it's been populated by another thread in the meantime




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to