hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ser...@apache.org
Subject [2/3] hive git commit: HIVE-13051 : Deadline class has numerous issues (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Date Tue, 23 Feb 2016 19:31:44 GMT
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/9f61fc69
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/9f61fc69
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/9f61fc69

Branch: refs/heads/master
Commit: 9f61fc69b9752667ea3e6253971caa7bd3157db3
Parents: 137b238
Author: Sergey Shelukhin <sershe@apache.org>
Authored: Tue Feb 23 11:28:52 2016 -0800
Committer: Sergey Shelukhin <sershe@apache.org>
Committed: Tue Feb 23 11:28:52 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/9f61fc69/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/9f61fc69/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/9f61fc69/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/9f61fc69/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));
+    }
   }
 }


Mime
View raw message