This is an automated email from the ASF dual-hosted git repository.
dkuzmenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 609b7a4b993 HIVE-27284: Make HMSHandler proxy pluggable (Wechar Yu,
reviewed by Denys Kuzmenko, Zhihua Deng)
609b7a4b993 is described below
commit 609b7a4b993570189385a74b1c6c77264e0883a7
Author: Wechar Yu <[email protected]>
AuthorDate: Sun Jun 25 01:30:52 2023 +0800
HIVE-27284: Make HMSHandler proxy pluggable (Wechar Yu, reviewed by Denys
Kuzmenko, Zhihua Deng)
Closes #4257
---
.../org/apache/iceberg/hive/TestHiveMetastore.java | 6 +-
.../hadoop/hive/metastore/HiveMetaStoreClient.java | 8 +-
.../hadoop/hive/metastore/conf/MetastoreConf.java | 5 ++
.../hadoop/hive/metastore/utils/JavaUtils.java | 4 +-
.../hive/metastore/AbstractHMSHandlerProxy.java | 96 ++++++++++++++++++++++
.../apache/hadoop/hive/metastore/HMSHandler.java | 22 -----
.../hive/metastore/HMSHandlerProxyFactory.java | 59 +++++++++++++
.../hadoop/hive/metastore/HiveMetaStore.java | 20 ++---
.../hadoop/hive/metastore/RetryingHMSHandler.java | 90 ++++----------------
.../hive/metastore/TestHiveMetaStoreTimeout.java | 56 +++++++++++--
.../metastore/TestRetriesInRetryingHMSHandler.java | 28 ++++---
.../hive/metastore/conf/TestMetastoreConf.java | 5 +-
12 files changed, 258 insertions(+), 141 deletions(-)
diff --git
a/iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
b/iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
index 76eb87b21b5..6af2e050755 100644
---
a/iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
+++
b/iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
@@ -29,9 +29,9 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.metastore.HMSHandler;
+import org.apache.hadoop.hive.metastore.HMSHandlerProxyFactory;
import org.apache.hadoop.hive.metastore.IHMSHandler;
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
-import org.apache.hadoop.hive.metastore.RetryingHMSHandler;
import org.apache.hadoop.hive.metastore.TSetIpAddressProcessor;
import org.apache.hadoop.hive.metastore.api.GetTableRequest;
import org.apache.hadoop.hive.metastore.api.Table;
@@ -67,8 +67,8 @@ public class TestHiveMetastore {
.build();
private static final DynMethods.StaticMethod GET_BASE_HMS_HANDLER =
DynMethods.builder("getProxy")
- .impl(RetryingHMSHandler.class, Configuration.class,
IHMSHandler.class, boolean.class)
- .impl(RetryingHMSHandler.class, HiveConf.class, IHMSHandler.class,
boolean.class)
+ .impl(HMSHandlerProxyFactory.class, Configuration.class,
IHMSHandler.class, boolean.class)
+ .impl(HMSHandlerProxyFactory.class, HiveConf.class,
IHMSHandler.class, boolean.class)
.buildStatic();
// Hive3 introduces background metastore tasks (MetastoreTaskThread) for
performing various cleanup duties. These
diff --git
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
index d3e3479899a..503183b4532 100644
---
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
+++
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
@@ -138,8 +138,8 @@ public class HiveMetaStoreClient implements
IMetaStoreClient, AutoCloseable {
"org.apache.hadoop.hive.metastore.HiveMetaStore";
// Method used to create Hive Metastore client. It is called as
- // HiveMetaStore.newRetryingHMSHandler("hive client", this.conf, true);
- private static final String HIVE_METASTORE_CREATE_HANDLER_METHOD =
"newRetryingHMSHandler";
+ // HiveMetaStore.newHMSHandler("hive client", this.conf, true);
+ private static final String HIVE_METASTORE_CREATE_HANDLER_METHOD =
"newHMSHandler";
ThriftHiveMetastore.Iface client = null;
private TTransport transport = null;
@@ -292,10 +292,10 @@ public class HiveMetaStoreClient implements
IMetaStoreClient, AutoCloseable {
//
// The code below simulates the following code
//
- // client = HiveMetaStore.newRetryingHMSHandler(this.conf);
+ // client = HiveMetaStore.newHMSHandler(this.conf);
//
// using reflection API. This is done to avoid dependency of
MetastoreClient on Hive Metastore.
- // Note that newRetryingHMSHandler is static method, so we pass null as
the object reference.
+ // Note that newHMSHandler is static method, so we pass null as the object
reference.
//
try {
Class<?> clazz = Class.forName(HIVE_METASTORE_CLASS);
diff --git
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
index 62ea1a63eae..c1354341c34 100644
---
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
+++
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
@@ -105,6 +105,8 @@ public class MetastoreConf {
public static final String
METASTORE_AUTHENTICATION_LDAP_USERMEMBERSHIPKEY_NAME =
"metastore.authentication.ldap.userMembershipKey";
+ public static final String METASTORE_RETRYING_HANDLER_CLASS =
+ "org.apache.hadoop.hive.metastore.RetryingHMSHandler";
private static final Map<String, ConfVars> metaConfs = new HashMap<>();
private static volatile URL hiveSiteURL = null;
@@ -874,6 +876,9 @@ public class MetastoreConf {
"testing only."),
HMS_HANDLER_INTERVAL("metastore.hmshandler.retry.interval",
"hive.hmshandler.retry.interval",
2000, TimeUnit.MILLISECONDS, "The time between HMSHandler retry
attempts on failure."),
+ HMS_HANDLER_PROXY_CLASS("metastore.hmshandler.proxy",
"hive.metastore.hmshandler.proxy",
+ METASTORE_RETRYING_HANDLER_CLASS,
+ "The proxy class name of HMSHandler, default is RetryingHMSHandler."),
IDENTIFIER_FACTORY("datanucleus.identifierFactory",
"datanucleus.identifierFactory", "datanucleus1",
"Name of the identifier factory to use when generating table/column
names etc. \n" +
diff --git
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/JavaUtils.java
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/JavaUtils.java
index b08d9fd71f6..503345d043d 100644
---
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/JavaUtils.java
+++
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/JavaUtils.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hive.metastore.utils;
+import org.apache.commons.lang3.ClassUtils;
import org.apache.hadoop.hive.metastore.api.MetaException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -71,7 +72,8 @@ public class JavaUtils {
"Number of constructor parameter types doesn't match number of
arguments");
}
for (int i = 0; i < parameterTypes.length; i++) {
- Class<?> clazz = parameterTypes[i];
+ // initargs are boxed to Object, so we need to wrapper primitive types
here.
+ Class<?> clazz = ClassUtils.primitiveToWrapper(parameterTypes[i]);
if (initargs[i] != null && !(clazz.isInstance(initargs[i]))) {
throw new IllegalArgumentException("Object : " + initargs[i]
+ " is not an instance of " + clazz);
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java
new file mode 100644
index 00000000000..539765c1514
--- /dev/null
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new
MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler,
boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes
applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf,
ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because
at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null,
metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static final class Result {
+ static final Result ERROR_RESULT = new Result(null, "error=true");
+ private final Object result;
+ private final String additionalInfo;
+
+ public Result(Object result, String additionalInfo) {
+ this.result = result;
+ this.additionalInfo = additionalInfo;
+ }
+ }
+
+ protected void initBaseHandler() throws MetaException {
+ baseHandler.init();
+ }
+
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] objects) throws
Throwable {
+ PerfLogger perfLogger = PerfLogger.getPerfLogger(false);
+ perfLogger.perfLogBegin(this.getClass().getName(), method.getName());
+ Result result = Result.ERROR_RESULT;
+ Deadline.registerIfNot(timeout);
+ try {
+ result = invokeInternal(proxy, method, objects);
+ return result == null ? null : result.result;
+ } finally {
+ String info = result == null ? "" : result.additionalInfo;
+ perfLogger.perfLogEnd(this.getClass().getName(), method.getName(), info);
+ }
+ }
+
+ protected abstract Result invokeInternal(Object proxy, Method method,
Object[] args)
+ throws Throwable;
+
+ public Configuration getActiveConf() {
+ return activeConf;
+ }
+}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
index 4f38f986b81..e0cc6486676 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
@@ -143,12 +143,6 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
public static final String PARTITION_NUMBER_EXCEED_LIMIT_MSG =
"Number of partitions scanned (=%d) on table '%s' exceeds limit (=%d).
This is controlled on the metastore server by %s.";
- // Used for testing to simulate method timeout.
- @VisibleForTesting
- static boolean testTimeoutEnabled = false;
- @VisibleForTesting
- static long testTimeoutValue = -1;
-
public static final String ADMIN = "admin";
public static final String PUBLIC = "public";
@@ -1345,14 +1339,6 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
// expected
}
- if (testTimeoutEnabled) {
- try {
- Thread.sleep(testTimeoutValue);
- } catch (InterruptedException e) {
- // do nothing
- }
- Deadline.checkTimeout();
- }
create_database_core(getMS(), db);
success = true;
} catch (Exception e) {
@@ -1892,14 +1878,6 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
} catch (NoSuchObjectException e) {
// expected
}
- if (testTimeoutEnabled) {
- try {
- Thread.sleep(testTimeoutValue);
- } catch (InterruptedException e) {
- // do nothing
- }
- Deadline.checkTimeout();
- }
create_dataconnector_core(getMS(), connector);
success = true;
} catch (Exception e) {
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandlerProxyFactory.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandlerProxyFactory.java
new file mode 100644
index 00000000000..b0a16ba71d2
--- /dev/null
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandlerProxyFactory.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.commons.lang3.exception.ExceptionUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+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;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Proxy;
+
+public class HMSHandlerProxyFactory {
+ private static final Logger LOG =
LoggerFactory.getLogger(HMSHandlerProxyFactory.class);
+
+ public static IHMSHandler getProxy(Configuration conf, IHMSHandler handler,
boolean local)
+ throws MetaException {
+ String hmsHandlerProxyName = MetastoreConf.getVar(conf,
ConfVars.HMS_HANDLER_PROXY_CLASS);
+ LOG.info("Creating HMSHandler proxy by class: {}", hmsHandlerProxyName);
+ Class<? extends AbstractHMSHandlerProxy> proxyClass =
+ JavaUtils.getClass(hmsHandlerProxyName,
AbstractHMSHandlerProxy.class);
+ AbstractHMSHandlerProxy invocationHandler = null;
+ try {
+ invocationHandler = JavaUtils.newInstance(proxyClass,
+ new Class[]{Configuration.class, IHMSHandler.class, boolean.class},
+ new Object[]{conf, handler, local});
+ } catch (Throwable t) {
+ // Reflection by JavaUtils will throw RuntimeException, try to get real
MetaException here.
+ Throwable rootCause = ExceptionUtils.getRootCause(t);
+ if (rootCause instanceof Exception) {
+ throw ExceptionHandler.newMetaException((Exception) rootCause);
+ }
+ throw t;
+ }
+
+ return (IHMSHandler) Proxy.newProxyInstance(
+ HMSHandlerProxyFactory.class.getClassLoader(),
+ new Class[]{ IHMSHandler.class }, invocationHandler);
+ }
+}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 40f5fe37991..4e49b76f6f9 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -136,18 +136,8 @@ public class HiveMetaStore extends ThriftHiveMetastore {
return true;
}
- private static IHMSHandler newRetryingHMSHandler(IHMSHandler baseHandler,
Configuration conf)
- throws MetaException {
- return newRetryingHMSHandler(baseHandler, conf, false);
- }
-
- private static IHMSHandler newRetryingHMSHandler(IHMSHandler baseHandler,
Configuration conf,
- boolean local) throws MetaException {
- return RetryingHMSHandler.getProxy(conf, baseHandler, local);
- }
-
/**
- * Create retrying HMS handler for embedded metastore.
+ * Create HMS handler for embedded metastore.
*
* <h1>IMPORTANT</h1>
*
@@ -158,10 +148,10 @@ public class HiveMetaStore extends ThriftHiveMetastore {
* @param conf configuration to use
* @throws MetaException
*/
- static Iface newRetryingHMSHandler(Configuration conf)
+ static Iface newHMSHandler(Configuration conf)
throws MetaException {
HMSHandler baseHandler = new HMSHandler("hive client", conf);
- return RetryingHMSHandler.getProxy(conf, baseHandler, true);
+ return HMSHandlerProxyFactory.getProxy(conf, baseHandler, true);
}
/**
@@ -453,7 +443,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
}
HMSHandler baseHandler = new HMSHandler("new db based metaserver", conf);
- IHMSHandler handler = newRetryingHMSHandler(baseHandler, conf);
+ IHMSHandler handler = HMSHandlerProxyFactory.getProxy(conf, baseHandler,
false);
processor = new ThriftHiveMetastore.Processor<>(handler);
LOG.info("Starting DB backed MetaStore Server with generic processor");
TServlet thriftHttpServlet = new HmsThriftHttpServlet(processor,
protocolFactory, conf);
@@ -564,7 +554,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
LOG.info("Binding host " + msHost + " for metastore server");
}
- IHMSHandler handler = newRetryingHMSHandler(baseHandler, conf);
+ IHMSHandler handler = HMSHandlerProxyFactory.getProxy(conf, baseHandler,
false);
TServerSocket serverSocket;
if (useSasl) {
processor = saslServer.wrapProcessor(
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
index 7774dfe5d46..ce7656fdb0c 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
@@ -18,10 +18,8 @@
package org.apache.hadoop.hive.metastore;
-import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
import java.lang.reflect.UndeclaredThrowableException;
import java.sql.SQLException;
import java.sql.SQLIntegrityConstraintViolationException;
@@ -35,7 +33,6 @@ import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
-import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
@@ -45,45 +42,23 @@ import org.datanucleus.exceptions.NucleusException;
@InterfaceAudience.Private
@InterfaceStability.Evolving
-public class RetryingHMSHandler implements InvocationHandler {
-
+public class RetryingHMSHandler extends AbstractHMSHandlerProxy {
private static final Logger LOG =
LoggerFactory.getLogger(RetryingHMSHandler.class);
- private static final String CLASS_NAME = RetryingHMSHandler.class.getName();
private static final Class<SQLException>[] unrecoverableSqlExceptions = new
Class[]{
// TODO: collect more unrecoverable SQLExceptions
SQLIntegrityConstraintViolationException.class
};
+ private final long retryInterval;
+ private final int retryLimit;
- private static class Result {
- private final Object result;
- private final int numRetries;
-
- public Result(Object result, int numRetries) {
- this.result = result;
- this.numRetries = numRetries;
- }
- }
-
- private final IHMSHandler baseHandler;
- private final MetaStoreInit.MetaStoreInitData metaStoreInitData =
- new MetaStoreInit.MetaStoreInitData();
-
- private final Configuration origConf; // base configuration
- private final Configuration activeConf; // active configuration
+ public RetryingHMSHandler(Configuration conf, IHMSHandler baseHandler,
boolean local)
+ throws MetaException {
+ super(conf, baseHandler, local);
+ retryInterval = MetastoreConf.getTimeVar(origConf,
+ ConfVars.HMS_HANDLER_INTERVAL, TimeUnit.MILLISECONDS);
+ retryLimit = MetastoreConf.getIntVar(origConf,
ConfVars.HMS_HANDLER_ATTEMPTS);
- private RetryingHMSHandler(Configuration origConf, IHMSHandler baseHandler,
boolean local) throws MetaException {
- this.origConf = origConf;
- this.baseHandler = baseHandler;
- if (local) {
- baseHandler.setConf(origConf); // tests expect configuration changes
applied directly to metastore
- }
- activeConf = baseHandler.getConf();
- // This has to be called before initializing the instance of HMSHandler
- // Using the hook on startup ensures that the hook always has priority
- // over settings in *.xml. The thread local conf needs to be used because
at this point
- // it has already been initialized using hiveConf.
- MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null,
metaStoreInitData);
try {
//invoking init method of baseHandler this way since it adds the retry
logic
//in case of transient failures in init method
@@ -97,46 +72,13 @@ public class RetryingHMSHandler implements
InvocationHandler {
}
}
- public static IHMSHandler getProxy(Configuration conf, IHMSHandler
baseHandler, boolean local)
- throws MetaException {
-
- RetryingHMSHandler handler = new RetryingHMSHandler(conf, baseHandler,
local);
-
- return (IHMSHandler) Proxy.newProxyInstance(
- RetryingHMSHandler.class.getClassLoader(),
- new Class[] { IHMSHandler.class }, handler);
- }
-
@Override
- public Object invoke(final Object proxy, final Method method, final Object[]
args) throws Throwable {
- int retryCount = -1;
- boolean error = true;
- PerfLogger perfLogger = PerfLogger.getPerfLogger(false);
- perfLogger.perfLogBegin(CLASS_NAME, method.getName());
- try {
- Result result = invokeInternal(proxy, method, args);
- retryCount = result.numRetries;
- error = false;
- return result.result;
- } finally {
- StringBuilder additionalInfo = new StringBuilder();
- additionalInfo.append("retryCount=").append(retryCount)
- .append(" error=").append(error);
- perfLogger.perfLogEnd(CLASS_NAME, method.getName(),
additionalInfo.toString());
- }
+ protected void initBaseHandler() throws MetaException {
+ // init operation has finished in constructor. noop here.
}
public Result invokeInternal(final Object proxy, final Method method, final
Object[] args) throws Throwable {
-
boolean gotNewConnectUrl = false;
- boolean reloadConf = MetastoreConf.getBoolVar(origConf,
ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
- long retryInterval = MetastoreConf.getTimeVar(origConf,
- ConfVars.HMS_HANDLER_INTERVAL, TimeUnit.MILLISECONDS);
- int retryLimit = MetastoreConf.getIntVar(origConf,
ConfVars.HMS_HANDLER_ATTEMPTS);
- long timeout = MetastoreConf.getTimeVar(origConf,
- ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
-
- Deadline.registerIfNot(timeout);
if (reloadConf) {
MetaStoreInit.updateConnectionURL(origConf, getActiveConf(),
@@ -159,8 +101,10 @@ public class RetryingHMSHandler implements
InvocationHandler {
Deadline.stopTimer();
}
}
- return new Result(object, retryCount);
-
+ StringBuilder additionalInfo = new StringBuilder();
+ additionalInfo.append("retryCount=").append(retryCount)
+ .append(" error=").append(false);
+ return new Result(object, additionalInfo.toString());
} catch (UndeclaredThrowableException e) {
if (e.getCause() != null) {
if (e.getCause() instanceof javax.jdo.JDOException) {
@@ -245,8 +189,4 @@ public class RetryingHMSHandler implements
InvocationHandler {
return Stream.of(unrecoverableSqlExceptions)
.allMatch(ex -> ExceptionUtils.indexOfType(t, ex) < 0);
}
-
- public Configuration getActiveConf() {
- return activeConf;
- }
}
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
index 7588f0d9941..66331ddcbfb 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java
@@ -18,8 +18,11 @@
package org.apache.hadoop.hive.metastore;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.UndeclaredThrowableException;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
+import java.lang.reflect.Method;
import java.util.concurrent.TimeUnit;
import org.apache.hadoop.conf.Configuration;
@@ -32,7 +35,6 @@ import
org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
import org.apache.thrift.TException;
import org.apache.thrift.transport.TTransportException;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
@@ -40,8 +42,7 @@ import org.junit.Test;
import org.junit.experimental.categories.Category;
/**
- * Test long running request timeout functionality in MetaStore Server
- * HMSHandler.create_database() is used to simulate a long running method.
+ * Test long running request timeout functionality in MetaStore Server.
*/
@Category(MetastoreCheckinTest.class)
public class TestHiveMetaStoreTimeout {
@@ -52,6 +53,42 @@ public class TestHiveMetaStoreTimeout {
private final String dbName = "db";
+ /** Test handler proxy used to simulate a long-running create_database()
method */
+ static class DelayedHMSHandler extends AbstractHMSHandlerProxy {
+ static long testTimeoutValue = -1;
+ public DelayedHMSHandler(Configuration conf, IHMSHandler baseHandler,
boolean local)
+ throws MetaException {
+ super(conf, baseHandler, local);
+ }
+
+ @Override
+ protected Result invokeInternal(Object proxy, Method method, Object[] args)
+ throws Throwable {
+ try {
+ boolean isStarted = Deadline.startTimer(method.getName());
+ Object object;
+ try {
+ if (testTimeoutValue > 0 &&
method.getName().equals("create_database")) {
+ try {
+ Thread.sleep(testTimeoutValue);
+ } catch (InterruptedException e) {
+ // do nothing.
+ }
+ Deadline.checkTimeout();
+ }
+ object = method.invoke(baseHandler, args);
+ } finally {
+ if (isStarted) {
+ Deadline.stopTimer();
+ }
+ }
+ return new Result(object, "error=false");
+ } catch (UndeclaredThrowableException | InvocationTargetException e) {
+ throw e.getCause();
+ }
+ }
+ }
+
@BeforeClass
public static void startMetaStoreServer() throws Exception {
conf = MetastoreConf.newMetastoreConf();
@@ -59,6 +96,7 @@ public class TestHiveMetaStoreTimeout {
MockPartitionExpressionForMetastore.class,
PartitionExpressionProxy.class);
MetastoreConf.setTimeVar(conf, ConfVars.CLIENT_SOCKET_TIMEOUT, 2000,
TimeUnit.MILLISECONDS);
+ MetastoreConf.setVar(conf, ConfVars.HMS_HANDLER_PROXY_CLASS,
DelayedHMSHandler.class.getName());
MetaStoreTestUtils.setConfForStandloneMode(conf);
warehouse = new Warehouse(conf);
port = MetaStoreTestUtils.startMetaStoreWithRetry(conf);
@@ -68,8 +106,7 @@ public class TestHiveMetaStoreTimeout {
@Before
public void setup() throws MetaException {
- HMSHandler.testTimeoutEnabled = false;
- HMSHandler.testTimeoutValue = -1;
+ DelayedHMSHandler.testTimeoutValue = -1;
client = new HiveMetaStoreClient(conf);
}
@@ -90,8 +127,7 @@ public class TestHiveMetaStoreTimeout {
@Test
public void testTimeout() throws Exception {
- HMSHandler.testTimeoutEnabled = true;
- HMSHandler.testTimeoutValue = 4000;
+ DelayedHMSHandler.testTimeoutValue = 4000;
Database db = new DatabaseBuilder()
.setName(dbName)
@@ -102,6 +138,9 @@ public class TestHiveMetaStoreTimeout {
} catch (TTransportException e) {
Assert.assertTrue("unexpected Exception", e.getMessage().contains("Read
timed out"));
}
+
+ // restore
+ DelayedHMSHandler.testTimeoutValue = -1;
}
@Test
@@ -117,8 +156,7 @@ public class TestHiveMetaStoreTimeout {
client.dropDatabase(dbName, true, true);
// reset
- HMSHandler.testTimeoutEnabled = true;
- HMSHandler.testTimeoutValue = 4000;
+ DelayedHMSHandler.testTimeoutValue = 4000;
// timeout after reset
try {
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java
index b970c54006f..4c4905deb0a 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRetriesInRetryingHMSHandler.java
@@ -48,6 +48,8 @@ public class TestRetriesInRetryingHMSHandler {
@BeforeClass
public static void setup() throws IOException {
conf = MetastoreConf.newMetastoreConf();
+ MetastoreConf.setVar(conf, ConfVars.HMS_HANDLER_PROXY_CLASS,
+ MetastoreConf.METASTORE_RETRYING_HANDLER_CLASS);
MetastoreConf.setLongVar(conf, ConfVars.HMS_HANDLER_ATTEMPTS,
RETRY_ATTEMPTS);
MetastoreConf.setTimeVar(conf, ConfVars.HMS_HANDLER_INTERVAL, 10,
TimeUnit.MILLISECONDS);
MetastoreConf.setBoolVar(conf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF,
false);
@@ -65,7 +67,7 @@ public class TestRetriesInRetryingHMSHandler {
.doThrow(JDOException.class)
.doNothing()
.when(mockBaseHandler).init();
- RetryingHMSHandler.getProxy(conf, mockBaseHandler, false);
+ HMSHandlerProxyFactory.getProxy(conf, mockBaseHandler, false);
Mockito.verify(mockBaseHandler, Mockito.times(2)).init();
}
@@ -77,7 +79,7 @@ public class TestRetriesInRetryingHMSHandler {
IHMSHandler mockBaseHandler = Mockito.mock(IHMSHandler.class);
Mockito.when(mockBaseHandler.getConf()).thenReturn(conf);
Mockito.doNothing().when(mockBaseHandler).init();
- RetryingHMSHandler.getProxy(conf, mockBaseHandler, false);
+ HMSHandlerProxyFactory.getProxy(conf, mockBaseHandler, false);
Mockito.verify(mockBaseHandler, Mockito.times(1)).init();
}
@@ -85,13 +87,18 @@ public class TestRetriesInRetryingHMSHandler {
* If the init method in HMSHandler throws exception all the times it should
be retried until
* HiveConf.ConfVars.HMSHANDLERATTEMPTS is reached before giving up
*/
- @Test(expected = MetaException.class)
+ @Test
public void testRetriesLimit() throws MetaException {
IHMSHandler mockBaseHandler = Mockito.mock(IHMSHandler.class);
Mockito.when(mockBaseHandler.getConf()).thenReturn(conf);
Mockito.doThrow(JDOException.class).when(mockBaseHandler).init();
- RetryingHMSHandler.getProxy(conf, mockBaseHandler, false);
- Mockito.verify(mockBaseHandler, Mockito.times(RETRY_ATTEMPTS)).init();
+ try {
+ HMSHandlerProxyFactory.getProxy(conf, mockBaseHandler, false);
+ Assert.fail("should fail for mockBaseHandler init.");
+ } catch (MetaException e) {
+ // expected
+ }
+ Mockito.verify(mockBaseHandler, Mockito.times(RETRY_ATTEMPTS + 1)).init();
}
/*
@@ -110,7 +117,7 @@ public class TestRetriesInRetryingHMSHandler {
.doThrow(me)
.doNothing()
.when(mockBaseHandler).init();
- RetryingHMSHandler.getProxy(conf, mockBaseHandler, false);
+ HMSHandlerProxyFactory.getProxy(conf, mockBaseHandler, false);
Mockito.verify(mockBaseHandler, Mockito.times(2)).init();
}
@@ -132,7 +139,7 @@ public class TestRetriesInRetryingHMSHandler {
InvocationTargetException ex = new InvocationTargetException(me);
Mockito.doThrow(me).when(mockBaseHandler).getMS();
- IHMSHandler retryingHandler = RetryingHMSHandler.getProxy(conf,
mockBaseHandler, false);
+ IHMSHandler retryingHandler = HMSHandlerProxyFactory.getProxy(conf,
mockBaseHandler, false);
try {
retryingHandler.getMS();
Assert.fail("should throw the mocked MetaException");
@@ -159,7 +166,7 @@ public class TestRetriesInRetryingHMSHandler {
InvocationTargetException ex = new InvocationTargetException(me);
Mockito.doThrow(me).when(mockBaseHandler).getMS();
- IHMSHandler retryingHandler = RetryingHMSHandler.getProxy(conf,
mockBaseHandler, false);
+ IHMSHandler retryingHandler = HMSHandlerProxyFactory.getProxy(conf,
mockBaseHandler, false);
try {
retryingHandler.getMS();
Assert.fail("should throw the mocked MetaException");
@@ -185,14 +192,13 @@ public class TestRetriesInRetryingHMSHandler {
InvocationTargetException ex = new InvocationTargetException(me);
Mockito.doThrow(me).when(mockBaseHandler).getMS();
- IHMSHandler retryingHandler = RetryingHMSHandler.getProxy(conf,
mockBaseHandler, false);
+ IHMSHandler retryingHandler = HMSHandlerProxyFactory.getProxy(conf,
mockBaseHandler, false);
try {
retryingHandler.getMS();
Assert.fail("should throw the mocked MetaException");
} catch (MetaException e) {
// expected
}
- int retryTimes = MetastoreConf.getIntVar(conf,
ConfVars.HMS_HANDLER_ATTEMPTS);
- Mockito.verify(mockBaseHandler, Mockito.times(retryTimes + 1)).getMS();
+ Mockito.verify(mockBaseHandler, Mockito.times(RETRY_ATTEMPTS + 1)).getMS();
}
}
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
index 92ab7f9eeed..a5a8145ccbd 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/conf/TestMetastoreConf.java
@@ -42,11 +42,12 @@ import java.util.Random;
import java.util.concurrent.TimeUnit;
import org.apache.hadoop.hive.metastore.DefaultStorageSchemaReader;
-import org.apache.hadoop.hive.metastore.SerDeStorageSchemaReader;
import org.apache.hadoop.hive.metastore.HiveAlterHandler;
import org.apache.hadoop.hive.metastore.MaterializationsRebuildLockCleanerTask;
import org.apache.hadoop.hive.metastore.MetastoreTaskThread;
+import org.apache.hadoop.hive.metastore.RetryingHMSHandler;
import org.apache.hadoop.hive.metastore.RuntimeStatsCleanerTask;
+import org.apache.hadoop.hive.metastore.SerDeStorageSchemaReader;
import org.apache.hadoop.hive.metastore.events.EventCleanerTask;
import
org.apache.hadoop.hive.metastore.security.MetastoreDelegationTokenManager;
import org.apache.hadoop.hive.metastore.txn.AcidHouseKeeperService;
@@ -491,6 +492,8 @@ public class TestMetastoreConf {
MaterializationsRebuildLockCleanerTask.class.getName());
Assert.assertEquals(MetastoreConf.METASTORE_TASK_THREAD_CLASS,
MetastoreTaskThread.class.getName());
+ Assert.assertEquals(MetastoreConf.METASTORE_RETRYING_HANDLER_CLASS,
+ RetryingHMSHandler.class.getName());
Assert.assertEquals(MetastoreConf.RUNTIME_STATS_CLEANER_TASK_CLASS,
RuntimeStatsCleanerTask.class.getName());
Assert.assertEquals(MetastoreConf.EVENT_CLEANER_TASK_CLASS,
