rdblue commented on a change in pull request #2129:
URL: https://github.com/apache/iceberg/pull/2129#discussion_r571149295



##########
File path: 
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalogs.java
##########
@@ -36,4 +38,11 @@ public static HiveCatalog loadCatalog(Configuration conf) {
     String metastoreUri = conf.get(HiveConf.ConfVars.METASTOREURIS.varname, 
"");
     return CATALOG_CACHE.get(metastoreUri, uri -> new HiveCatalog(conf));
   }
+
+  public static HiveCatalog loadCatalog(String catalogName, Map<String, 
String> properties, Configuration conf) {
+    // metastore URI can be null in local mode
+    String metastoreUri = conf.get(HiveConf.ConfVars.METASTOREURIS.varname, 
"");
+    return CATALOG_CACHE.get(metastoreUri, uri -> (HiveCatalog) 
CatalogUtil.loadCatalog(HiveCatalog.class.getName(),
+            catalogName, properties, conf));
+  }

Review comment:
       The concern about the number of connections to HMS makes sense, but we 
should check some assumptions:
   1. Are you saying that there will be 2 on HS2 and 2 on the AM because the 
pool size is 2? If there aren't concurrent uses, then the pool won't actually 
grow to the maximum size. Maybe the AM only has 1?
   2. Is more than one connection abnormal? If so, Hive could set the pool size 
to 1.
   3. Does table serialization help here to avoid using the pool on the AM?
   4. Can we share client pools across catalogs?
   
   I"d rather take care of this problem using stricter settings.




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