You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by li...@apache.org on 2014/04/04 20:18:32 UTC
svn commit: r1584853 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/ipc/thrift/
main/java/org/apache/hadoop/hbase/regionserver/
main/java/org/apache/hadoop/hbase/util/
test/java/org/apache/hadoop/hbase/thrift/swift/
Author: liyin
Date: Fri Apr 4 18:18:32 2014
New Revision: 1584853
URL: http://svn.apache.org/r1584853
Log:
[HBASE-9930] Closing the connections in few missed cases in the HBaseToThriftAdapter
Author: manukranthk
Summary: This diff attempts to close all the ThriftClientManager and Thrift client objects on the client side.
Test Plan: Minor change.
Reviewers: gauravm, adela, fan, elliott
Reviewed By: elliott
CC: hbase-eng@, elliott, ehwang
Differential Revision: https://phabricator.fb.com/D1253630
Task ID: 4024967
Added:
hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/swift/TestJVMCrash.java
Modified:
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseThriftRPC.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseToThriftAdapter.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftClientCacheWithConnectionPooling.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseThriftRPC.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseThriftRPC.java?rev=1584853&r1=1584852&r2=1584853&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseThriftRPC.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseThriftRPC.java Fri Apr 4 18:18:32 2014
@@ -30,6 +30,8 @@ import org.apache.hadoop.hbase.util.Pair
import java.io.IOException;
import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
import java.util.Stack;
import java.util.concurrent.ConcurrentHashMap;
@@ -119,8 +121,9 @@ public class HBaseThriftRPC {
HBaseRPCOptions options) throws IOException {
Pair<ThriftClientInterface, ThriftClientManager> interfaceAndManager = getClientWithoutWrapper(
addr, conf, clazz);
- return new HBaseToThriftAdapter(interfaceAndManager.getFirst(),
+ HBaseToThriftAdapter ret = new HBaseToThriftAdapter(interfaceAndManager.getFirst(),
interfaceAndManager.getSecond(), addr, conf, clazz, options);
+ return ret;
}
/**
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseToThriftAdapter.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseToThriftAdapter.java?rev=1584853&r1=1584852&r2=1584853&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseToThriftAdapter.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/HBaseToThriftAdapter.java Fri Apr 4 18:18:32 2014
@@ -78,8 +78,8 @@ import com.google.common.util.concurrent
*/
public class HBaseToThriftAdapter implements HRegionInterface {
public static final Log LOG = LogFactory.getLog(HBaseToThriftAdapter.class);
- private ThriftHRegionInterface connection;
- private ThriftClientManager clientManager;
+ public ThriftHRegionInterface connection;
+ public ThriftClientManager clientManager;
private InetSocketAddress addr;
private Configuration conf;
private Class<? extends ThriftClientInterface> clazz;
@@ -118,7 +118,8 @@ public class HBaseToThriftAdapter implem
} else if (e instanceof RuntimeTApplicationException) {
throw new RuntimeException(e);
} else {
- //TODO: creating a new connection is unnecessary. We should replace it with cleanUpConnection later
+ //TODO: creating a new connection is unnecessary.
+ // We should replace it with cleanUpConnection later
Pair<ThriftClientInterface, ThriftClientManager> interfaceAndManager = HBaseThriftRPC
.refreshConnection(this.addr, this.conf, this.connection, this.clazz);
this.connection = (ThriftHRegionInterface) interfaceAndManager.getFirst();
@@ -179,7 +180,14 @@ public class HBaseToThriftAdapter implem
}
private void preProcess() {
- if (this.connection == null || this.clientManager == null) {
+ if (this.connection == null || clientManager == null) {
+ if (connection != null) {
+ try {
+ connection.close();
+ } catch (Exception e) {
+ LOG.error("Could not close connection : " + connection, e);
+ }
+ }
try {
Pair<ThriftClientInterface, ThriftClientManager> clientAndManager = HBaseThriftRPC
.getClientWithoutWrapper(addr, conf, clazz);
@@ -193,8 +201,7 @@ public class HBaseToThriftAdapter implem
try {
setHeader();
} catch (Exception e) {
- e.printStackTrace();
- LOG.error("Header could not be sent");
+ LOG.error("Header could not be sent", e);
}
}
}
@@ -300,7 +307,11 @@ public class HBaseToThriftAdapter implem
@Override
public void close() throws Exception {
- throw new UnsupportedOperationException("Method is not supported for thrift");
+ clientManager = null;
+ if (this.connection != null) {
+ this.connection.close();
+ this.connection = null;
+ }
}
@Override
@@ -585,6 +596,8 @@ public class HBaseToThriftAdapter implem
} catch (Exception e) {
refreshConnectionAndThrowIOException(e);
return false;
+ } finally {
+ postProcess();
}
}
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftClientCacheWithConnectionPooling.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftClientCacheWithConnectionPooling.java?rev=1584853&r1=1584852&r2=1584853&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftClientCacheWithConnectionPooling.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftClientCacheWithConnectionPooling.java Fri Apr 4 18:18:32 2014
@@ -184,6 +184,7 @@ public class ThriftClientCacheWithConnec
closeAll(pair.getFirst(), pair.getSecond());
}
clientPools.clear();
+ this.clientManager.close();
}
private void closeAll(InetSocketAddress server,
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1584853&r1=1584852&r2=1584853&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Fri Apr 4 18:18:32 2014
@@ -4078,6 +4078,7 @@ public class HRegionServer implements HR
@Override
public void close() throws Exception {
+ this.shutdownServers();
}
@Override
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java?rev=1584853&r1=1584852&r2=1584853&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java Fri Apr 4 18:18:32 2014
@@ -209,6 +209,22 @@ public class JVMClusterUtil {
if (interrupted) {
Thread.currentThread().interrupt();
}
+ for (Thread t : regionservers) {
+ try {
+ ((RegionServerThread)t).regionServer.close();
+ } catch (Exception e) {
+ LOG.error("Could not close regionserver resources");
+ e.printStackTrace();
+ }
+ }
+ for (HMaster t : masters) {
+ try {
+ t.close();
+ } catch (Exception e) {
+ LOG.error("Could not close master resources");
+ e.printStackTrace();
+ }
+ }
}
LOG.info("Shutdown of " +
Added: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/swift/TestJVMCrash.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/swift/TestJVMCrash.java?rev=1584853&view=auto
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/swift/TestJVMCrash.java (added)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/swift/TestJVMCrash.java Fri Apr 4 18:18:32 2014
@@ -0,0 +1,19 @@
+package org.apache.hadoop.hbase.thrift.swift;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.junit.Test;
+
+public class TestJVMCrash {
+ private HBaseTestingUtility hbaseTestingUtility = new HBaseTestingUtility();
+ private final Log LOG = LogFactory.getLog(TestJVMCrash.class);
+ @Test
+ public void testRepeatedHBaseTestingUtilityRuns() throws Exception {
+ for (int i = 0; i < 10; i++) {
+ LOG.debug("Iteration : " + i);
+ hbaseTestingUtility.startMiniCluster(2);
+ hbaseTestingUtility.shutdownMiniCluster();
+ }
+ }
+}