You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2016/03/09 20:58:15 UTC
[2/2] hive git commit: HIVE-13051 : Deadline class has numerous
issues (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
HIVE-13051 : Deadline class has numerous issues (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/23d15583
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/23d15583
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/23d15583
Branch: refs/heads/branch-2.0
Commit: 23d1558330e6cfdc78cc88aad4fa4c83cc7de62e
Parents: 864bdc4
Author: Sergey Shelukhin <se...@apache.org>
Authored: Tue Feb 23 11:28:52 2016 -0800
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Wed Mar 9 11:57:51 2016 -0800
----------------------------------------------------------------------
.../apache/hadoop/hive/metastore/Deadline.java | 63 +++++++++-----------
.../hadoop/hive/metastore/RawStoreProxy.java | 28 +++------
.../hive/metastore/RetryingHMSHandler.java | 12 +++-
.../metastore/SessionPropertiesListener.java | 9 +--
4 files changed, 50 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/23d15583/metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java b/metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java
index f29d453..71d336a 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/Deadline.java
@@ -32,34 +32,33 @@ public class Deadline {
/**
* its value is init from conf, and could be reset from client.
*/
- private long timeout;
+ private long timeoutNanos;
/**
* it is reset before executing a method
*/
- private long startTime = -1;
+ private long startTime = NO_DEADLINE;
/**
* The name of public methods in HMSHandler
*/
private String method;
- private Deadline(long timeout) {
- this.timeout = timeout;
+ private Deadline(long timeoutMs) {
+ this.timeoutNanos = timeoutMs * 1000000L;
}
/**
* Deadline object per thread.
*/
- private static final ThreadLocal<Deadline> DEADLINE_THREAD_LOCAL = new
- ThreadLocal<Deadline>() {
+ private static final ThreadLocal<Deadline> DEADLINE_THREAD_LOCAL = new ThreadLocal<Deadline>() {
@Override
protected Deadline initialValue() {
return null;
}
};
- static void setCurrentDeadline(Deadline deadline) {
+ private static void setCurrentDeadline(Deadline deadline) {
DEADLINE_THREAD_LOCAL.set(deadline);
}
@@ -67,7 +66,7 @@ public class Deadline {
return DEADLINE_THREAD_LOCAL.get();
}
- static void removeCurrentDeadline() {
+ private static void removeCurrentDeadline() {
DEADLINE_THREAD_LOCAL.remove();
}
@@ -85,29 +84,14 @@ public class Deadline {
* reset the timeout value of this timer.
* @param timeout
*/
- public static void resetTimeout(long timeout) throws MetaException {
- if (timeout <= 0) {
+ public static void resetTimeout(long timeoutMs) throws MetaException {
+ if (timeoutMs <= 0) {
throw newMetaException(new DeadlineException("The reset timeout value should be " +
- "larger than 0: " + timeout));
+ "larger than 0: " + timeoutMs));
}
Deadline deadline = getCurrentDeadline();
if (deadline != null) {
- deadline.timeout = timeout;
- } else {
- throw newMetaException(new DeadlineException("The threadlocal Deadline is null," +
- " please register it firstly."));
- }
- }
-
- /**
- * Check whether the timer is started.
- * @return
- * @throws MetaException
- */
- public static boolean isStarted() throws MetaException {
- Deadline deadline = getCurrentDeadline();
- if (deadline != null) {
- return deadline.startTime >= 0;
+ deadline.timeoutNanos = timeoutMs * 1000000L;
} else {
throw newMetaException(new DeadlineException("The threadlocal Deadline is null," +
" please register it firstly."));
@@ -118,15 +102,18 @@ public class Deadline {
* start the timer before a method is invoked.
* @param method
*/
- public static void startTimer(String method) throws MetaException {
+ public static boolean startTimer(String method) throws MetaException {
Deadline deadline = getCurrentDeadline();
- if (deadline != null) {
- deadline.startTime = System.currentTimeMillis();
- deadline.method = method;
- } else {
+ if (deadline == null) {
throw newMetaException(new DeadlineException("The threadlocal Deadline is null," +
" please register it firstly."));
}
+ if (deadline.startTime != NO_DEADLINE) return false;
+ deadline.method = method;
+ do {
+ deadline.startTime = System.nanoTime();
+ } while (deadline.startTime == NO_DEADLINE);
+ return true;
}
/**
@@ -135,7 +122,7 @@ public class Deadline {
public static void stopTimer() throws MetaException {
Deadline deadline = getCurrentDeadline();
if (deadline != null) {
- deadline.startTime = -1;
+ deadline.startTime = NO_DEADLINE;
deadline.method = null;
} else {
throw newMetaException(new DeadlineException("The threadlocal Deadline is null," +
@@ -164,14 +151,18 @@ public class Deadline {
}
}
+ private static final long NO_DEADLINE = Long.MIN_VALUE;
+
private void check() throws MetaException{
try {
- if (startTime < 0) {
+ if (startTime == NO_DEADLINE) {
throw new DeadlineException("Should execute startTimer() method before " +
"checkTimeout. Error happens in method: " + method);
}
- if (startTime + timeout < System.currentTimeMillis()) {
- throw new DeadlineException("Timeout when executing method: " + method);
+ long elapsedTime = System.nanoTime() - startTime;
+ if (elapsedTime > timeoutNanos) {
+ throw new DeadlineException("Timeout when executing method: " + method + "; "
+ + (elapsedTime / 1000000L) + "ms exceeds " + (timeoutNanos / 1000000L) + "ms");
}
} catch (DeadlineException e) {
throw newMetaException(e);
http://git-wip-us.apache.org/repos/asf/hive/blob/23d15583/metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java
index f28e232..c5e117d 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java
@@ -43,11 +43,14 @@ public class RawStoreProxy implements InvocationHandler {
new MetaStoreInit.MetaStoreInitData();
private final HiveConf hiveConf;
private final Configuration conf; // thread local conf from HMS
+ private final long socketTimeout;
protected RawStoreProxy(HiveConf hiveConf, Configuration conf,
Class<? extends RawStore> rawStoreClass, int id) throws MetaException {
this.conf = conf;
this.hiveConf = hiveConf;
+ this.socketTimeout = HiveConf.getTimeVar(hiveConf,
+ HiveConf.ConfVars.METASTORE_CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
// This has to be called before initializing the instance of RawStore
init();
@@ -91,34 +94,21 @@ public class RawStoreProxy implements InvocationHandler {
@Override
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
- Object ret = null;
- boolean isTimerStarted = false;
-
try {
+ Deadline.registerIfNot(socketTimeout);
+ boolean isTimerStarted = Deadline.startTimer(method.getName());
try {
- if (!Deadline.isStarted()) {
- Deadline.startTimer(method.getName());
- isTimerStarted = true;
+ return method.invoke(base, args);
+ } finally {
+ if (isTimerStarted) {
+ Deadline.stopTimer();
}
- } catch (MetaException e) {
- // Deadline was not registered yet.
- long timeout = HiveConf.getTimeVar(hiveConf,
- HiveConf.ConfVars.METASTORE_CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
- Deadline.registerIfNot(timeout);
- Deadline.startTimer(method.getName());
- isTimerStarted = true;
- }
- ret = method.invoke(base, args);
-
- if (isTimerStarted) {
- Deadline.stopTimer();
}
} catch (UndeclaredThrowableException e) {
throw e.getCause();
} catch (InvocationTargetException e) {
throw e.getCause();
}
- return ret;
}
public Configuration getConf() {
http://git-wip-us.apache.org/repos/asf/hive/blob/23d15583/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
index f01849d..2fc487f 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
@@ -134,9 +134,15 @@ public class RetryingHMSHandler implements InvocationHandler {
if (reloadConf || gotNewConnectUrl) {
baseHandler.setConf(getActiveConf());
}
- Deadline.startTimer(method.getName());
- Object object = method.invoke(baseHandler, args);
- Deadline.stopTimer();
+ Object object = null;
+ boolean isStarted = Deadline.startTimer(method.getName());
+ try {
+ object = method.invoke(baseHandler, args);
+ } finally {
+ if (isStarted) {
+ Deadline.stopTimer();
+ }
+ }
return new Result(object, retryCount);
} catch (javax.jdo.JDOException e) {
http://git-wip-us.apache.org/repos/asf/hive/blob/23d15583/metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java b/metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
index d16cab0..ee96678 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java
@@ -36,9 +36,10 @@ public class SessionPropertiesListener extends MetaStoreEventListener {
@Override
public void onConfigChange(ConfigChangeEvent changeEvent) throws MetaException {
- if (changeEvent.getKey().equals(HiveConf.ConfVars.METASTORE_CLIENT_SOCKET_TIMEOUT.varname)) {
- Deadline.resetTimeout(HiveConf.toTime(changeEvent.getNewValue(), TimeUnit.SECONDS,
- TimeUnit.MILLISECONDS));
- }
+ if (changeEvent.getKey().equals(HiveConf.ConfVars.METASTORE_CLIENT_SOCKET_TIMEOUT.varname)) {
+ // TODO: this only applies to current thread, so it's not useful at all.
+ Deadline.resetTimeout(HiveConf.toTime(changeEvent.getNewValue(), TimeUnit.SECONDS,
+ TimeUnit.MILLISECONDS));
+ }
}
}