hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sp...@apache.org
Subject hive git commit: HIVE-14187: JDOPersistenceManager objects remain cached if MetaStoreClient#close is not called (Mohit Sabharwal, reviewed by Vaibhav Gumasha, via Sergio Pena)
Date Fri, 15 Jul 2016 21:47:22 GMT
Repository: hive
Updated Branches:
  refs/heads/master a1d6b2de0 -> 12c8de048


HIVE-14187: JDOPersistenceManager objects remain cached if MetaStoreClient#close is not called
(Mohit Sabharwal, reviewed by Vaibhav Gumasha, via Sergio Pena)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/12c8de04
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/12c8de04
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/12c8de04

Branch: refs/heads/master
Commit: 12c8de048631dd69a27d62ba2f4038134db4df11
Parents: a1d6b2d
Author: Mohit Sabharwal <mohit@cloudera.com>
Authored: Fri Jul 15 16:46:04 2016 -0500
Committer: Sergio Pena <sergio.pena@cloudera.com>
Committed: Fri Jul 15 16:46:04 2016 -0500

----------------------------------------------------------------------
 .../hive/metastore/TestHiveMetaStore.java       | 51 ++++++++++++++++++++
 .../hive/metastore/TestRemoteHiveMetaStore.java | 10 ++--
 .../hive/metastore/TestSetUGIOnOnlyClient.java  |  4 +-
 .../hive/metastore/TestSetUGIOnOnlyServer.java  |  4 +-
 .../hadoop/hive/metastore/HiveMetaStore.java    | 35 ++++++++------
 .../hive/metastore/HiveMetaStoreClient.java     |  6 +++
 6 files changed, 88 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
index ddfaacf..d90085b 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hive.metastore;
 
+import java.lang.reflect.Field;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
@@ -33,6 +34,8 @@ import java.util.Set;
 
 import junit.framework.TestCase;
 
+import org.datanucleus.api.jdo.JDOPersistenceManager;
+import org.datanucleus.api.jdo.JDOPersistenceManagerFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.hadoop.fs.FileSystem;
@@ -3211,6 +3214,54 @@ public abstract class TestHiveMetaStore extends TestCase {
     client.close();
   }
 
+  public void testJDOPersistanceManagerCleanup() throws Exception {
+    if (isThriftClient == false) {
+      return;
+    }
+
+    int numObjectsBeforeClose =  getJDOPersistanceManagerCacheSize();
+    HiveMetaStoreClient closingClient = new HiveMetaStoreClient(hiveConf);
+    closingClient.getAllDatabases();
+    closingClient.close();
+    Thread.sleep(5 * 1000); // give HMS time to handle close request
+    int numObjectsAfterClose =  getJDOPersistanceManagerCacheSize();
+    Assert.assertTrue(numObjectsBeforeClose == numObjectsAfterClose);
+
+    HiveMetaStoreClient nonClosingClient = new HiveMetaStoreClient(hiveConf);
+    nonClosingClient.getAllDatabases();
+    // Drop connection without calling close. HMS thread deleteContext
+    // will trigger cleanup
+    nonClosingClient.getTTransport().close();
+    Thread.sleep(5 * 1000);
+    int numObjectsAfterDroppedConnection =  getJDOPersistanceManagerCacheSize();
+    Assert.assertTrue(numObjectsAfterClose == numObjectsAfterDroppedConnection);
+  }
+
+  private static int getJDOPersistanceManagerCacheSize() {
+    JDOPersistenceManagerFactory jdoPmf;
+    Set<JDOPersistenceManager> pmCacheObj;
+    Field pmCache;
+    Field pmf;
+    try {
+      pmf = ObjectStore.class.getDeclaredField("pmf");
+      if (pmf != null) {
+        pmf.setAccessible(true);
+        jdoPmf = (JDOPersistenceManagerFactory) pmf.get(null);
+        pmCache = JDOPersistenceManagerFactory.class.getDeclaredField("pmCache");
+        if (pmCache != null) {
+          pmCache.setAccessible(true);
+          pmCacheObj = (Set<JDOPersistenceManager>) pmCache.get(jdoPmf);
+          if (pmCacheObj != null) {
+            return pmCacheObj.size();
+          }
+        }
+      }
+    } catch (Exception ex) {
+      System.out.println(ex);
+    }
+    return -1;
+  }
+
   private HiveMetaHookLoader getHookLoader() {
     HiveMetaHookLoader hookLoader = new HiveMetaHookLoader() {
       @Override

http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java
index 6da5165..ef02968 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java
@@ -25,6 +25,7 @@ import org.apache.hadoop.hive.shims.ShimLoader;
 
 public class TestRemoteHiveMetaStore extends TestHiveMetaStore {
   private static boolean isServerStarted = false;
+  private static int port;
 
   public TestRemoteHiveMetaStore() {
     super();
@@ -37,21 +38,22 @@ public class TestRemoteHiveMetaStore extends TestHiveMetaStore {
 
     if (isServerStarted) {
       assertNotNull("Unable to connect to the MetaStore server", client);
+      hiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + port);
       return;
     }
 
-    int port = MetaStoreUtils.findFreePort();
+    port = MetaStoreUtils.findFreePort();
     System.out.println("Starting MetaStore Server on port " + port);
     MetaStoreUtils.startMetaStore(port, ShimLoader.getHadoopThriftAuthBridge(), hiveConf);
     isServerStarted = true;
 
     // This is default case with setugi off for both client and server
-    createClient(false, port);
+    createClient(false);
   }
 
-  protected void createClient(boolean setugi, int port) throws Exception {
+  protected void createClient(boolean setugi) throws Exception {
     hiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, "thrift://localhost:" + port);
     hiveConf.setBoolVar(ConfVars.METASTORE_EXECUTE_SET_UGI,setugi);
     client = new HiveMetaStoreClient(hiveConf);
   }
-}
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java
index 2c6d567..29768c1 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java
@@ -21,8 +21,8 @@ package org.apache.hadoop.hive.metastore;
 public class TestSetUGIOnOnlyClient extends TestRemoteHiveMetaStore{
 
   @Override
-  protected void createClient(boolean setugi, int port) throws Exception {
+  protected void createClient(boolean setugi) throws Exception {
     // turn it on for client.
-    super.createClient(true, port);
+    super.createClient(true);
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java
index 6c3fbf6..4a46f75 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java
@@ -21,8 +21,8 @@ package org.apache.hadoop.hive.metastore;
 public class TestSetUGIOnOnlyServer extends TestSetUGIOnBothClientServer {
 
   @Override
-  protected void createClient(boolean setugi, int port) throws Exception {
+  protected void createClient(boolean setugi) throws Exception {
     // It is turned on for both client and server because of super class. Turn it off for
client.
-    super.createClient(false, port);
+    super.createClient(false);
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index c6c1e11..38c0eed 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -284,7 +284,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
           }
         };
 
-    private final void logAuditEvent(String cmd) {
+    private static final void logAuditEvent(String cmd) {
       if (cmd == null) {
         return;
       }
@@ -307,7 +307,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
           address, cmd).toString());
     }
 
-    String getIPAddress() {
+    private static String getIPAddress() {
       if (useSasl) {
         if (saslServer != null && saslServer.getRemoteAddress() != null) {
           return saslServer.getRemoteAddress().getHostAddress();
@@ -750,7 +750,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       }
     }
 
-    private void logInfo(String m) {
+    private static void logInfo(String m) {
       LOG.info(threadLocalId.get().toString() + ": " + m);
       logAuditEvent(m);
     }
@@ -823,17 +823,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
 
     @Override
     public void shutdown() {
-      logInfo("Metastore shutdown started...");
-      RawStore ms = threadLocalMS.get();
-      if (ms != null) {
-        try {
-          ms.shutdown();
-        } finally {
-          threadLocalConf.remove();
-          threadLocalMS.remove();
-        }
-      }
-      logInfo("Metastore shutdown complete.");
+      cleanupRawStore();
     }
 
     @Override
@@ -6833,6 +6823,9 @@ public class HiveMetaStore extends ThriftHiveMetastore {
           } catch (Exception e) {
             LOG.warn("Error Reporting Metastore close connection to Metrics system", e);
           }
+          // If the IMetaStoreClient#close was called, HMSHandler#shutdown would have already
+          // cleaned up thread local RawStore. Otherwise, do it now.
+          cleanupRawStore();
         }
 
         @Override
@@ -6860,6 +6853,20 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     }
   }
 
+  private static void cleanupRawStore() {
+    RawStore rs = HMSHandler.getRawStore();
+    if (rs != null) {
+      HMSHandler.logInfo("Cleaning up thread local RawStore...");
+      try {
+        rs.shutdown();
+      } finally {
+        HMSHandler.threadLocalConf.remove();
+        HMSHandler.removeRawStore();
+      }
+      HMSHandler.logInfo("Done cleaning up thread local RawStore");
+    }
+  }
+
   private static void signalOtherThreadsToStart(final TServer server, final Lock startLock,
                                                 final Condition startCondition,
                                                 final AtomicBoolean startedServing) {

http://git-wip-us.apache.org/repos/asf/hive/blob/12c8de04/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
index 44d73d4..909d8eb 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hive.metastore;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hive.common.ObjectPair;
 import org.apache.hadoop.hive.common.ValidTxnList;
 import org.apache.hadoop.hive.common.classification.InterfaceAudience;
@@ -326,6 +327,11 @@ public class HiveMetaStoreClient implements IMetaStoreClient {
     metastoreUris[index] = tmp;
   }
 
+  @VisibleForTesting
+  public TTransport getTTransport() {
+    return transport;
+  }
+
   @Override
   public boolean isLocalMetaStore() {
     return localMetaStore;


Mime
View raw message