Skip to content

Commit b8dc73f

Browse files
dengzhhu653dengzhhu653
authored
HIVE-29620: RetryingMetaStoreClient uses HiveMetaStoreClientBuilder t… (#6499)
1 parent 341b863 commit b8dc73f

6 files changed

Lines changed: 79 additions & 62 deletions

File tree

‎iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/client/HiveRESTCatalogClient.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,6 @@ public class HiveRESTCatalogClient extends BaseMetaStoreClient {
6868

6969
private RESTCatalog restCatalog;
7070

71-
public HiveRESTCatalogClient(Configuration conf, boolean allowEmbedded) {
72-
this(conf);
73-
}
74-
7571
public HiveRESTCatalogClient(Configuration conf) {
7672
super(conf);
7773
reconnect();

‎ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6094,8 +6094,7 @@ public HiveMetaHook getHook(
60946094
}
60956095
};
60966096

6097-
HiveMetaStoreClientBuilder msClientBuilder = new HiveMetaStoreClientBuilder(conf)
6098-
.newClient(allowEmbedded)
6097+
HiveMetaStoreClientBuilder msClientBuilder = new HiveMetaStoreClientBuilder(conf, allowEmbedded)
60996098
.enhanceWith(client ->
61006099
HiveMetaStoreClientWithLocalCache.newClient(conf, client))
61016100
.enhanceWith(client ->

‎standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader) th
6161

6262
public HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader, Boolean allowEmbedded)
6363
throws MetaException {
64-
this(conf, hookLoader, new HiveMetaStoreClientBuilder(conf).newClient(allowEmbedded).build());
64+
this(conf, hookLoader, new HiveMetaStoreClientBuilder(conf, allowEmbedded).build());
6565
}
6666

6767
private HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader,
@@ -75,8 +75,7 @@ private HiveMetaStoreClient(Configuration conf, HiveMetaHookLoader hookLoader,
7575

7676
private static IMetaStoreClient createUnderlyingClient(Configuration conf, HiveMetaHookLoader hookLoader,
7777
IMetaStoreClient baseMetaStoreClient) {
78-
return new HiveMetaStoreClientBuilder(conf)
79-
.client(baseMetaStoreClient)
78+
return new HiveMetaStoreClientBuilder(conf, baseMetaStoreClient)
8079
.withHooks(hookLoader)
8180
.threadSafe()
8281
.build();

‎standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,12 @@ default void setHiveAddedJars(String addedJars) {
6565

6666
/**
6767
* Returns true if the current client is using an in process metastore (local metastore).
68+
* Default false, as in real production the client should always connect to a remote meta service
6869
*
6970
* @return
7071
*/
71-
default boolean isLocalMetaStore(){
72-
throw new UnsupportedOperationException("MetaStore client does not support checking if metastore is local");
72+
default boolean isLocalMetaStore() {
73+
return false;
7374
}
7475

7576
/**

‎standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java‎

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@
3333
import java.util.concurrent.TimeUnit;
3434
import java.util.function.Supplier;
3535

36+
import org.apache.commons.lang3.ClassUtils;
3637
import org.apache.hadoop.classification.InterfaceAudience;
3738
import org.apache.hadoop.conf.Configuration;
3839
import org.apache.hadoop.hive.common.classification.RetrySemantics;
40+
import org.apache.hadoop.hive.metastore.client.builder.HiveMetaStoreClientBuilder;
3941
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
4042
import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
4143
import org.apache.hadoop.hive.metastore.utils.JavaUtils;
@@ -69,15 +71,7 @@ public class RetryingMetaStoreClient implements InvocationHandler {
6971
private final Map<String, Long> metaCallTimeMap;
7072
private final long connectionLifeTimeInMillis;
7173
private long lastConnectionTime;
72-
private boolean localMetaStore;
73-
74-
75-
protected RetryingMetaStoreClient(Configuration conf, Class<?>[] constructorArgTypes,
76-
Object[] constructorArgs, Map<String, Long> metaCallTimeMap,
77-
Class<? extends IMetaStoreClient> msClientClass) throws MetaException {
78-
this(conf, metaCallTimeMap, () ->
79-
JavaUtils.newInstance(msClientClass, constructorArgTypes, constructorArgs));
80-
}
74+
private final boolean localMetaStore;
8175

8276
protected RetryingMetaStoreClient(Configuration conf, Map<String, Long> metaCallTimeMap,
8377
Supplier<? extends IMetaStoreClient> msClient) throws MetaException {
@@ -95,12 +89,11 @@ protected RetryingMetaStoreClient(Configuration conf, Map<String, Long> metaCall
9589
this.connectionLifeTimeInMillis = MetastoreConf.getTimeVar(conf,
9690
ConfVars.CLIENT_SOCKET_LIFETIME, TimeUnit.MILLISECONDS);
9791
this.lastConnectionTime = System.currentTimeMillis();
98-
String msUri = MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS);
99-
localMetaStore = (msUri == null) || msUri.trim().isEmpty();
10092

10193
SecurityUtils.reloginExpiringKeytabUser();
10294

10395
this.base = msClient.get();
96+
this.localMetaStore = base.isLocalMetaStore();
10497

10598
LOG.info("RetryingMetaStoreClient proxy=" + base.getClass() + " ugi=" + this.ugi
10699
+ " retries=" + this.retryLimit + " delay=" + this.retryDelaySeconds
@@ -109,9 +102,7 @@ protected RetryingMetaStoreClient(Configuration conf, Map<String, Long> metaCall
109102

110103
public static IMetaStoreClient getProxy(
111104
Configuration hiveConf, boolean allowEmbedded) throws MetaException {
112-
return getProxy(hiveConf, new Class[]{Configuration.class, HiveMetaHookLoader.class, Boolean.class},
113-
new Object[]{hiveConf, null, allowEmbedded}, null, HiveMetaStoreClient.class.getName()
114-
);
105+
return new HiveMetaStoreClientBuilder(hiveConf, allowEmbedded).withRetry(null).build();
115106
}
116107

117108
@VisibleForTesting
@@ -123,13 +114,10 @@ public static IMetaStoreClient getProxy(Configuration hiveConf, HiveMetaHookLoad
123114
public static IMetaStoreClient getProxy(Configuration hiveConf, HiveMetaHookLoader hookLoader,
124115
Map<String, Long> metaCallTimeMap, String mscClassName, boolean allowEmbedded)
125116
throws MetaException {
126-
127-
return getProxy(hiveConf,
128-
new Class[] {Configuration.class, HiveMetaHookLoader.class, Boolean.class},
129-
new Object[] {hiveConf, hookLoader, allowEmbedded},
130-
metaCallTimeMap,
131-
mscClassName
132-
);
117+
return
118+
new HiveMetaStoreClientBuilder(hiveConf, mscClassName, allowEmbedded)
119+
.withHooks(hookLoader)
120+
.withRetry(metaCallTimeMap).build();
133121
}
134122

135123
/**
@@ -148,26 +136,18 @@ public static IMetaStoreClient getProxy(Configuration hiveConf, Class<?>[] const
148136
public static IMetaStoreClient getProxy(Configuration hiveConf, Class<?>[] constructorArgTypes,
149137
Object[] constructorArgs, Map<String, Long> metaCallTimeMap,
150138
String mscClassName) throws MetaException {
151-
152139
@SuppressWarnings("unchecked")
153140
Class<? extends IMetaStoreClient> baseClass =
154141
JavaUtils.getClass(mscClassName, IMetaStoreClient.class);
155-
156-
RetryingMetaStoreClient handler =
157-
new RetryingMetaStoreClient(hiveConf, constructorArgTypes, constructorArgs,
158-
metaCallTimeMap, baseClass);
159-
return getProxy(baseClass.getInterfaces(), handler);
142+
IMetaStoreClient baseClient = JavaUtils.newInstance(baseClass, constructorArgTypes, constructorArgs);
143+
return new HiveMetaStoreClientBuilder(hiveConf, baseClient).withRetry(metaCallTimeMap).build();
160144
}
161145

162146
public static IMetaStoreClient getProxy(Configuration hiveConf, Map<String, Long> metaCallTimeMap,
163147
IMetaStoreClient msClient) throws MetaException {
164148
RetryingMetaStoreClient handler =
165149
new RetryingMetaStoreClient(hiveConf, metaCallTimeMap, () -> msClient);
166-
return getProxy(msClient.getClass().getInterfaces(), handler);
167-
}
168-
169-
private static IMetaStoreClient getProxy(Class<?>[] interfaces,
170-
RetryingMetaStoreClient handler) {
150+
Class<?>[] interfaces = ClassUtils.getAllInterfaces(msClient.getClass()).toArray(new Class[0]);
171151
return (IMetaStoreClient) Proxy.newProxyInstance(
172152
RetryingMetaStoreClient.class.getClassLoader(), interfaces, handler);
173153
}

‎standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java‎

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package org.apache.hadoop.hive.metastore.client.builder;
2020

2121
import org.apache.commons.lang3.exception.ExceptionUtils;
22+
import org.apache.commons.lang3.reflect.ConstructorUtils;
23+
import org.apache.commons.lang3.tuple.Pair;
2224
import org.apache.hadoop.conf.Configuration;
2325
import org.apache.hadoop.hive.metastore.HiveMetaHookLoader;
2426
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
@@ -32,28 +34,43 @@
3234
import org.slf4j.Logger;
3335
import org.slf4j.LoggerFactory;
3436

37+
import java.lang.reflect.Constructor;
3538
import java.util.Map;
3639
import java.util.Objects;
40+
import java.util.concurrent.ConcurrentHashMap;
3741
import java.util.function.Function;
3842

3943
public class HiveMetaStoreClientBuilder {
4044
private static final Logger LOG = LoggerFactory.getLogger(HiveMetaStoreClientBuilder.class);
45+
private static final Map<Class<? extends IMetaStoreClient>, MetaStoreClientFactory>
46+
CLIENT_FACTORIES = new ConcurrentHashMap<>();
4147

4248
private final Configuration conf;
4349
private IMetaStoreClient client;
4450

45-
public HiveMetaStoreClientBuilder(Configuration conf) {
46-
this.conf = Objects.requireNonNull(conf);
51+
public HiveMetaStoreClientBuilder(Configuration configuration) throws MetaException {
52+
this(configuration, true);
4753
}
4854

49-
public HiveMetaStoreClientBuilder newClient(boolean allowEmbedded) throws MetaException {
50-
this.client = createClient(conf, allowEmbedded);
51-
return this;
55+
public HiveMetaStoreClientBuilder(Configuration configuration, boolean allowEmbedded) throws MetaException {
56+
this.conf = Objects.requireNonNull(configuration);
57+
Class<? extends IMetaStoreClient> mscClass = MetastoreConf.getClass(
58+
conf, MetastoreConf.ConfVars.METASTORE_CLIENT_IMPL,
59+
ThriftHiveMetaStoreClient.class, IMetaStoreClient.class);
60+
this.client = createClient(conf, mscClass, allowEmbedded);
5261
}
5362

54-
public HiveMetaStoreClientBuilder client(IMetaStoreClient client) {
55-
this.client = client;
56-
return this;
63+
public HiveMetaStoreClientBuilder(Configuration configuration, String clientImpl,
64+
boolean allowEmbedded) throws MetaException {
65+
this.conf = Objects.requireNonNull(configuration);
66+
Class<? extends IMetaStoreClient> baseClass =
67+
JavaUtils.getClass(clientImpl, IMetaStoreClient.class);
68+
this.client = createClient(configuration, baseClass, allowEmbedded);
69+
}
70+
71+
public HiveMetaStoreClientBuilder(Configuration configuration, IMetaStoreClient client) {
72+
this.conf = Objects.requireNonNull(configuration);
73+
this.client = Objects.requireNonNull(client);
5774
}
5875

5976
public HiveMetaStoreClientBuilder enhanceWith(Function<IMetaStoreClient, IMetaStoreClient> wrapperFunction) {
@@ -80,17 +97,15 @@ public IMetaStoreClient build() {
8097
return Objects.requireNonNull(client);
8198
}
8299

83-
private static IMetaStoreClient createClient(Configuration conf, boolean allowEmbedded) throws MetaException {
84-
Class<? extends IMetaStoreClient> mscClass = MetastoreConf.getClass(
85-
conf, MetastoreConf.ConfVars.METASTORE_CLIENT_IMPL,
86-
ThriftHiveMetaStoreClient.class, IMetaStoreClient.class);
87-
LOG.info("Using {} as a base MetaStoreClient", mscClass.getName());
88-
89-
IMetaStoreClient baseMetaStoreClient = null;
100+
private static IMetaStoreClient createClient(Configuration conf,
101+
Class<? extends IMetaStoreClient> mscClass, boolean allowEmbedded) throws MetaException {
90102
try {
91-
baseMetaStoreClient = JavaUtils.newInstance(mscClass,
92-
new Class[]{Configuration.class, boolean.class},
93-
new Object[]{conf, allowEmbedded});
103+
LOG.info("Using {} as a base MetaStoreClient", mscClass.getName());
104+
MetaStoreClientFactory factory = CLIENT_FACTORIES.get(mscClass);
105+
if (factory == null) {
106+
CLIENT_FACTORIES.put(mscClass, factory = new MetaStoreClientFactory(mscClass));
107+
}
108+
return factory.createClient(conf, allowEmbedded);
94109
} catch (Throwable t) {
95110
// Reflection by JavaUtils will throw RuntimeException, try to get real MetaException here.
96111
Throwable rootCause = ExceptionUtils.getRootCause(t);
@@ -100,7 +115,34 @@ private static IMetaStoreClient createClient(Configuration conf, boolean allowEm
100115
throw new MetaException(rootCause.getMessage());
101116
}
102117
}
118+
}
103119

104-
return baseMetaStoreClient;
120+
private static class MetaStoreClientFactory {
121+
private Constructor<? extends IMetaStoreClient> bestMatchingCtr;
122+
private Function<Pair<Configuration, Boolean>, Object[]> argsTransformer;
123+
124+
MetaStoreClientFactory(Class<? extends IMetaStoreClient> mscClass) {
125+
Constructor<? extends IMetaStoreClient> candidate =
126+
ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class, boolean.class);
127+
if (candidate != null) {
128+
this.bestMatchingCtr = candidate;
129+
this.argsTransformer = args -> new Object[] {args.getLeft(), (boolean) args.getRight()};
130+
} else if ((candidate = ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class,
131+
HiveMetaHookLoader.class, Boolean.class)) != null) {
132+
this.bestMatchingCtr = candidate;
133+
this.argsTransformer = args ->
134+
new Object[] {args.getLeft(), null, Boolean.valueOf(args.getRight())};
135+
} else if ((candidate = ConstructorUtils.getMatchingAccessibleConstructor(mscClass, Configuration.class)) != null) {
136+
this.bestMatchingCtr = candidate;
137+
this.argsTransformer = args -> new Object[] {args.getLeft()};
138+
}
139+
if (bestMatchingCtr == null) {
140+
throw new RuntimeException("No matching constructor found for this IMetaStoreClient " + mscClass);
141+
}
142+
}
143+
144+
IMetaStoreClient createClient(Configuration conf, boolean allowEmbedded) throws Exception {
145+
return bestMatchingCtr.newInstance(argsTransformer.apply(Pair.of(conf, allowEmbedded)));
146+
}
105147
}
106148
}

0 commit comments

Comments
 (0)