diff --git a/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/client/HiveRESTCatalogClient.java b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/client/HiveRESTCatalogClient.java index 4390d5a0bca1..a0ca7c937aa9 100644 --- a/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/client/HiveRESTCatalogClient.java +++ b/iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/client/HiveRESTCatalogClient.java @@ -68,10 +68,6 @@ public class HiveRESTCatalogClient extends BaseMetaStoreClient { private RESTCatalog restCatalog; - public HiveRESTCatalogClient(Configuration conf, boolean allowEmbedded) { - this(conf); - } - public HiveRESTCatalogClient(Configuration conf) { super(conf); reconnect(); diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java index cd33896807bc..31f695ed558b 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java @@ -6094,8 +6094,7 @@ public HiveMetaHook getHook( } }; - HiveMetaStoreClientBuilder msClientBuilder = new HiveMetaStoreClientBuilder(conf) - .newClient(allowEmbedded) + HiveMetaStoreClientBuilder msClientBuilder = new HiveMetaStoreClientBuilder(conf, allowEmbedded) .enhanceWith(client -> HiveMetaStoreClientWithLocalCache.newClient(conf, client)) .enhanceWith(client -> diff --git a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java index 122c0c3c491d..75defe50ebbe 100644 --- a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java +++ b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java @@ -61,7 +61,7 @@ public HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader) th public HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader, Boolean allowEmbedded) throws MetaException { - this(conf, hookLoader, new HiveMetaStoreClientBuilder(conf).newClient(allowEmbedded).build()); + this(conf, hookLoader, new HiveMetaStoreClientBuilder(conf, allowEmbedded).build()); } private HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader, @@ -75,8 +75,7 @@ private HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader, private static IMetaStoreClient createUnderlyingClient(Configuration conf, HiveMetaHookLoader hookLoader, IMetaStoreClient baseMetaStoreClient) { - return new HiveMetaStoreClientBuilder(conf) - .client(baseMetaStoreClient) + return new HiveMetaStoreClientBuilder(conf, baseMetaStoreClient) .withHooks(hookLoader) .threadSafe() .build(); diff --git a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java index 7b03f7f096e8..5b7b174c26e7 100644 --- a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java +++ b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java @@ -65,11 +65,12 @@ default void setHiveAddedJars(String addedJars) { /** * Returns true if the current client is using an in process metastore (local metastore). + * Default false, as in real production the client should always connect to a remote meta service * * @return */ - default boolean isLocalMetaStore(){ - throw new UnsupportedOperationException("MetaStore client does not support checking if metastore is local"); + default boolean isLocalMetaStore() { + return false; } /** diff --git a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java index 0cf9901fd2ad..14f6ba3c3427 100644 --- a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java +++ b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java @@ -33,9 +33,11 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; +import org.apache.commons.lang3.ClassUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.common.classification.RetrySemantics; +import org.apache.hadoop.hive.metastore.client.builder.HiveMetaStoreClientBuilder; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; import org.apache.hadoop.hive.metastore.utils.JavaUtils; @@ -69,15 +71,7 @@ public class RetryingMetaStoreClient implements InvocationHandler { private final Map metaCallTimeMap; private final long connectionLifeTimeInMillis; private long lastConnectionTime; - private boolean localMetaStore; - - - protected RetryingMetaStoreClient(Configuration conf, Class[] constructorArgTypes, - Object[] constructorArgs, Map metaCallTimeMap, - Class msClientClass) throws MetaException { - this(conf, metaCallTimeMap, () -> - JavaUtils.newInstance(msClientClass, constructorArgTypes, constructorArgs)); - } + private final boolean localMetaStore; protected RetryingMetaStoreClient(Configuration conf, Map metaCallTimeMap, Supplier msClient) throws MetaException { @@ -95,12 +89,11 @@ protected RetryingMetaStoreClient(Configuration conf, Map metaCall this.connectionLifeTimeInMillis = MetastoreConf.getTimeVar(conf, ConfVars.CLIENT_SOCKET_LIFETIME, TimeUnit.MILLISECONDS); this.lastConnectionTime = System.currentTimeMillis(); - String msUri = MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS); - localMetaStore = (msUri == null) || msUri.trim().isEmpty(); SecurityUtils.reloginExpiringKeytabUser(); this.base = msClient.get(); + this.localMetaStore = base.isLocalMetaStore(); LOG.info("RetryingMetaStoreClient proxy=" + base.getClass() + " ugi=" + this.ugi + " retries=" + this.retryLimit + " delay=" + this.retryDelaySeconds @@ -109,9 +102,7 @@ protected RetryingMetaStoreClient(Configuration conf, Map metaCall public static IMetaStoreClient getProxy( Configuration hiveConf, boolean allowEmbedded) throws MetaException { - return getProxy(hiveConf, new Class[]{Configuration.class, HiveMetaHookLoader.class, Boolean.class}, - new Object[]{hiveConf, null, allowEmbedded}, null, HiveMetaStoreClient.class.getName() - ); + return new HiveMetaStoreClientBuilder(hiveConf, allowEmbedded).withRetry(null).build(); } @VisibleForTesting @@ -123,13 +114,10 @@ public static IMetaStoreClient getProxy(Configuration hiveConf, HiveMetaHookLoad public static IMetaStoreClient getProxy(Configuration hiveConf, HiveMetaHookLoader hookLoader, Map metaCallTimeMap, String mscClassName, boolean allowEmbedded) throws MetaException { - - return getProxy(hiveConf, - new Class[] {Configuration.class, HiveMetaHookLoader.class, Boolean.class}, - new Object[] {hiveConf, hookLoader, allowEmbedded}, - metaCallTimeMap, - mscClassName - ); + return + new HiveMetaStoreClientBuilder(hiveConf, mscClassName, allowEmbedded) + .withHooks(hookLoader) + .withRetry(metaCallTimeMap).build(); } /** @@ -148,26 +136,18 @@ public static IMetaStoreClient getProxy(Configuration hiveConf, Class[] const public static IMetaStoreClient getProxy(Configuration hiveConf, Class[] constructorArgTypes, Object[] constructorArgs, Map metaCallTimeMap, String mscClassName) throws MetaException { - @SuppressWarnings("unchecked") Class baseClass = JavaUtils.getClass(mscClassName, IMetaStoreClient.class); - - RetryingMetaStoreClient handler = - new RetryingMetaStoreClient(hiveConf, constructorArgTypes, constructorArgs, - metaCallTimeMap, baseClass); - return getProxy(baseClass.getInterfaces(), handler); + IMetaStoreClient baseClient = JavaUtils.newInstance(baseClass, constructorArgTypes, constructorArgs); + return new HiveMetaStoreClientBuilder(hiveConf, baseClient).withRetry(metaCallTimeMap).build(); } public static IMetaStoreClient getProxy(Configuration hiveConf, Map metaCallTimeMap, IMetaStoreClient msClient) throws MetaException { RetryingMetaStoreClient handler = new RetryingMetaStoreClient(hiveConf, metaCallTimeMap, () -> msClient); - return getProxy(msClient.getClass().getInterfaces(), handler); - } - - private static IMetaStoreClient getProxy(Class[] interfaces, - RetryingMetaStoreClient handler) { + Class[] interfaces = ClassUtils.getAllInterfaces(msClient.getClass()).toArray(new Class[0]); return (IMetaStoreClient) Proxy.newProxyInstance( RetryingMetaStoreClient.class.getClassLoader(), interfaces, handler); } diff --git a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java index 903a3543ee29..fbf72220082b 100644 --- a/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java +++ b/standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hive.metastore.client.builder; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.commons.lang3.reflect.ConstructorUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.metastore.HiveMetaHookLoader; import org.apache.hadoop.hive.metastore.IMetaStoreClient; @@ -32,28 +34,39 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.lang.reflect.Constructor; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; public class HiveMetaStoreClientBuilder { private static final Logger LOG = LoggerFactory.getLogger(HiveMetaStoreClientBuilder.class); + private static final Map, MetaStoreClientFactory> + CLIENT_FACTORIES = new ConcurrentHashMap<>(); private final Configuration conf; private IMetaStoreClient client; - public HiveMetaStoreClientBuilder(Configuration conf) { - this.conf = Objects.requireNonNull(conf); + public HiveMetaStoreClientBuilder(Configuration configuration, boolean allowEmbedded) throws MetaException { + this.conf = Objects.requireNonNull(configuration); + Class mscClass = MetastoreConf.getClass( + conf, MetastoreConf.ConfVars.METASTORE_CLIENT_IMPL, + ThriftHiveMetaStoreClient.class, IMetaStoreClient.class); + this.client = createClient(conf, mscClass, allowEmbedded); } - public HiveMetaStoreClientBuilder newClient(boolean allowEmbedded) throws MetaException { - this.client = createClient(conf, allowEmbedded); - return this; + public HiveMetaStoreClientBuilder(Configuration configuration, String clientImpl, + boolean allowEmbedded) throws MetaException { + this.conf = Objects.requireNonNull(configuration); + Class baseClass = + JavaUtils.getClass(clientImpl, IMetaStoreClient.class); + this.client = createClient(configuration, baseClass, allowEmbedded); } - public HiveMetaStoreClientBuilder client(IMetaStoreClient client) { - this.client = client; - return this; + public HiveMetaStoreClientBuilder(Configuration configuration, IMetaStoreClient client) { + this.conf = Objects.requireNonNull(configuration); + this.client = Objects.requireNonNull(client); } public HiveMetaStoreClientBuilder enhanceWith(Function wrapperFunction) { @@ -80,17 +93,15 @@ public IMetaStoreClient build() { return Objects.requireNonNull(client); } - private static IMetaStoreClient createClient(Configuration conf, boolean allowEmbedded) throws MetaException { - Class mscClass = MetastoreConf.getClass( - conf, MetastoreConf.ConfVars.METASTORE_CLIENT_IMPL, - ThriftHiveMetaStoreClient.class, IMetaStoreClient.class); - LOG.info("Using {} as a base MetaStoreClient", mscClass.getName()); - - IMetaStoreClient baseMetaStoreClient = null; + private static IMetaStoreClient createClient(Configuration conf, + Class mscClass, boolean allowEmbedded) throws MetaException { try { - baseMetaStoreClient = JavaUtils.newInstance(mscClass, - new Class[]{Configuration.class, boolean.class}, - new Object[]{conf, allowEmbedded}); + LOG.info("Using {} as a base MetaStoreClient", mscClass.getName()); + MetaStoreClientFactory factory = CLIENT_FACTORIES.get(mscClass); + if (factory == null) { + CLIENT_FACTORIES.put(mscClass, factory = new MetaStoreClientFactory(mscClass)); + } + return factory.createClient(conf, allowEmbedded); } catch (Throwable t) { // Reflection by JavaUtils will throw RuntimeException, try to get real MetaException here. Throwable rootCause = ExceptionUtils.getRootCause(t); @@ -100,7 +111,34 @@ private static IMetaStoreClient createClient(Configuration conf, boolean allowEm throw new MetaException(rootCause.getMessage()); } } + } + + private static class MetaStoreClientFactory { + private Constructor bestMatchingCtr; + private Function, Object[]> argsTransformer; + + MetaStoreClientFactory(Class mscClass) { + Constructor candidate = + ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class, boolean.class); + if (candidate != null) { + this.bestMatchingCtr = candidate; + this.argsTransformer = args -> new Object[] {args.getLeft(), (boolean) args.getRight()}; + } else if ((candidate = ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class, + HiveMetaHookLoader.class, Boolean.class)) != null) { + this.bestMatchingCtr = candidate; + this.argsTransformer = args -> + new Object[] {args.getLeft(), null, Boolean.valueOf(args.getRight())}; + } else if ((candidate = ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class)) != null) { + this.bestMatchingCtr = candidate; + this.argsTransformer = args -> new Object[] {args.getLeft()}; + } + if (bestMatchingCtr == null) { + throw new RuntimeException("No matching constructor found for this IMetaStoreClient " + mscClass); + } + } - return baseMetaStoreClient; + IMetaStoreClient createClient(Configuration conf, boolean allowEmbedded) throws Exception { + return bestMatchingCtr.newInstance(argsTransformer.apply(Pair.of(conf, allowEmbedded))); + } } }