You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by sp...@apache.org on 2016/07/15 21:47:22 UTC
hive git commit: HIVE-14187: JDOPersistenceManager objects remain
cached if MetaStoreClient#close is not called (Mohit Sabharwal,
reviewed by Vaibhav Gumasha, via Sergio Pena)
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 <mo...@cloudera.com>
Authored: Fri Jul 15 16:46:04 2016 -0500
Committer: Sergio Pena <se...@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;