From 99dd95cfabcb275c7f339dc06a0a21d29a48bd9f Mon Sep 17 00:00:00 2001 From: Mikhail Antonov Date: Wed, 25 Mar 2015 14:40:17 -0700 Subject: [PATCH] HBASE-13252 Get rid of managed connections and connection caching --- .../org/apache/hadoop/hbase/MetaTableAccessor.java | 13 -- .../hadoop/hbase/client/ClusterConnection.java | 6 - .../hadoop/hbase/client/ConnectionAdapter.java | 5 - .../hadoop/hbase/client/ConnectionFactory.java | 12 +- .../hbase/client/ConnectionImplementation.java | 39 +----- .../hadoop/hbase/client/ConnectionManager.java | 141 ++------------------ .../hadoop/hbase/client/ConnectionUtils.java | 4 +- .../apache/hadoop/hbase/client/HConnection.java | 14 +- .../hadoop/hbase/client/TestClientNoCluster.java | 16 +-- .../hbase/client/CoprocessorHConnection.java | 4 +- .../hbase/ServerResourceCheckerJUnitListener.java | 15 --- .../hbase/client/HConnectionTestingUtility.java | 50 ++----- .../org/apache/hadoop/hbase/client/TestAdmin2.java | 20 --- .../org/apache/hadoop/hbase/client/TestHCM.java | 148 +-------------------- 14 files changed, 38 insertions(+), 449 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index 4df58a2..0e0adf2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -253,19 +253,6 @@ public class MetaTableAccessor { if (connection == null || connection.isClosed()) { throw new NullPointerException("No connection"); } - // If the passed in 'connection' is 'managed' -- i.e. every second test uses - // a Table or an HBaseAdmin with managed connections -- then doing - // connection.getTable will throw an exception saying you are NOT to use - // managed connections getting tables. Leaving this as it is for now. Will - // revisit when inclined to change all tests. User code probaby makes use of - // managed connections too so don't change it till post hbase 1.0. - // - // There should still be a way to use this method with an unmanaged connection. - if (connection instanceof ClusterConnection) { - if (((ClusterConnection) connection).isManaged()) { - throw new NeedUnmanagedConnectionException(); - } - } return connection.getTable(TableName.META_TABLE_NAME); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClusterConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClusterConnection.java index f0398f9..e3b7f92 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClusterConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ClusterConnection.java @@ -288,12 +288,6 @@ public interface ClusterConnection extends HConnection { RpcRetryingCallerFactory getNewRpcRetryingCallerFactory(Configuration conf); /** - * - * @return true if this is a managed connection. - */ - boolean isManaged(); - - /** * @return the current statistics tracker associated with this connection */ ServerStatisticTracker getStatisticsTracker(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionAdapter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionAdapter.java index e1458b8..09aef5c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionAdapter.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionAdapter.java @@ -456,11 +456,6 @@ abstract class ConnectionAdapter implements ClusterConnection { } @Override - public boolean isManaged() { - return wrappedConnection.isManaged(); - } - - @Override public ServerStatisticTracker getStatisticsTracker() { return wrappedConnection.getStatisticsTracker(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java index fd23d58..3e8ca31 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionFactory.java @@ -214,15 +214,9 @@ public class ConnectionFactory { user = provider.getCurrent(); } - return createConnection(conf, false, pool, user); - } - - static Connection createConnection(final Configuration conf, final boolean managed, - final ExecutorService pool, final User user) - throws IOException { String className = conf.get(HConnection.HBASE_CLIENT_CONNECTION_IMPL, ConnectionImplementation.class.getName()); - Class clazz = null; + Class clazz; try { clazz = Class.forName(className); } catch (ClassNotFoundException e) { @@ -232,9 +226,9 @@ public class ConnectionFactory { // Default HCM#HCI is not accessible; make it so before invoking. Constructor constructor = clazz.getDeclaredConstructor(Configuration.class, - boolean.class, ExecutorService.class, User.class); + ExecutorService.class, User.class); constructor.setAccessible(true); - return (Connection) constructor.newInstance(conf, managed, pool, user); + return (Connection) constructor.newInstance(conf, pool, user); } catch (Exception e) { throw new IOException(e); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java index 93ddea9..f69ac0a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java @@ -149,9 +149,6 @@ class ConnectionImplementation implements ClusterConnection, Closeable { private int refCount; - // indicates whether this connection's life cycle is managed (by us) - private boolean managed; - private User user; private RpcRetryingCallerFactory rpcCallerFactory; @@ -167,27 +164,15 @@ class ConnectionImplementation implements ClusterConnection, Closeable { private final ClientBackoffPolicy backoffPolicy; - ConnectionImplementation(Configuration conf, boolean managed) throws IOException { - this(conf, managed, null, null); - } - /** * constructor * @param conf Configuration object - * @param managed If true, does not do full shutdown on close; i.e. cleanup of connection - * to zk and shutdown of all services; we just close down the resources this connection was - * responsible for and decrement usage counters. It is up to the caller to do the full - * cleanup. It is set when we want have connection sharing going on -- reuse of zk connection, - * and cached region locations, established regionserver connections, etc. When connections - * are shared, we have reference counting going on and will only do full cleanup when no more - * users of an ConnectionImplementation instance. */ - ConnectionImplementation(Configuration conf, boolean managed, + ConnectionImplementation(Configuration conf, ExecutorService pool, User user) throws IOException { this(conf); this.user = user; this.batchPool = pool; - this.managed = managed; this.registry = setupRegistry(); retrieveClusterId(); @@ -221,6 +206,7 @@ class ConnectionImplementation implements ClusterConnection, Closeable { /** * For tests. */ + @VisibleForTesting protected ConnectionImplementation(Configuration conf) { this.conf = conf; this.tableConfig = new TableConfiguration(conf); @@ -292,9 +278,6 @@ class ConnectionImplementation implements ClusterConnection, Closeable { @Override public HTableInterface getTable(TableName tableName, ExecutorService pool) throws IOException { - if (managed) { - throw new NeedUnmanagedConnectionException(); - } return new HTable(tableName, this, tableConfig, rpcCallerFactory, rpcControllerFactory, pool); } @@ -327,9 +310,6 @@ class ConnectionImplementation implements ClusterConnection, Closeable { @Override public Admin getAdmin() throws IOException { - if (managed) { - throw new NeedUnmanagedConnectionException(); - } return new HBaseAdmin(this); } @@ -1955,15 +1935,7 @@ class ConnectionImplementation implements ClusterConnection, Closeable { @Override public void close() { - if (managed) { - if (aborted) { - ConnectionManager.deleteStaleConnection(this); - } else { - ConnectionManager.deleteConnection(this, false); - } - } else { - internalClose(); - } + internalClose(); } /** @@ -2128,9 +2100,4 @@ class ConnectionImplementation implements ClusterConnection, Closeable { return RpcRetryingCallerFactory .instantiate(conf, this.interceptor, this.getStatisticsTracker()); } - - @Override - public boolean isManaged() { - return managed; - } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java index 4eacf7b..c64fad6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java @@ -20,9 +20,6 @@ package org.apache.hadoop.hbase.client; import java.io.IOException; import java.util.Date; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; @@ -31,7 +28,6 @@ import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.RegionTooBusyException; import org.apache.hadoop.hbase.ServerName; @@ -44,6 +40,7 @@ import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.ExceptionUtil; import org.apache.hadoop.ipc.RemoteException; +import org.apache.hadoop.mapreduce.Cluster; /** * An internal, non-instantiable class that manages creation of {@link HConnection}s. @@ -56,30 +53,6 @@ final class ConnectionManager { public static final String RETRIES_BY_SERVER_KEY = "hbase.client.retries.by.server"; - // An LRU Map of HConnectionKey -> HConnection (TableServer). All - // access must be synchronized. This map is not private because tests - // need to be able to tinker with it. - static final Map CONNECTION_INSTANCES; - - public static final int MAX_CACHED_CONNECTION_INSTANCES; - - static { - // We set instances to one more than the value specified for {@link - // HConstants#ZOOKEEPER_MAX_CLIENT_CNXNS}. By default, the zk default max - // connections to the ensemble from the one client is 30, so in that case we - // should run into zk issues before the LRU hit this value of 31. - MAX_CACHED_CONNECTION_INSTANCES = HBaseConfiguration.create().getInt( - HConstants.ZOOKEEPER_MAX_CLIENT_CNXNS, HConstants.DEFAULT_ZOOKEPER_MAX_CLIENT_CNXNS) + 1; - CONNECTION_INSTANCES = new LinkedHashMap( - (int) (MAX_CACHED_CONNECTION_INSTANCES / 0.75F) + 1, 0.75F, true) { - @Override - protected boolean removeEldestEntry( - Map.Entry eldest) { - return size() > MAX_CACHED_CONNECTION_INSTANCES; - } - }; - } - /** Dummy nonce generator for disabled nonces. */ static class NoNonceGenerator implements NonceGenerator { @Override @@ -99,38 +72,8 @@ final class ConnectionManager { super(); } - /** - * Get the connection that goes with the passed conf configuration instance. - * If no current connection exists, method creates a new connection and keys it using - * connection-specific properties from the passed {@link Configuration}; see - * {@link HConnectionKey}. - * @param conf configuration - * @return HConnection object for conf - * @throws ZooKeeperConnectionException - * @deprecated connection caching is going away. - */ - @Deprecated - public static HConnection getConnection(final Configuration conf) throws IOException { - return getConnectionInternal(conf); - } - - - static ClusterConnection getConnectionInternal(final Configuration conf) - throws IOException { - HConnectionKey connectionKey = new HConnectionKey(conf); - synchronized (CONNECTION_INSTANCES) { - ConnectionImplementation connection = CONNECTION_INSTANCES.get(connectionKey); - if (connection == null) { - connection = (ConnectionImplementation) ConnectionFactory.createConnection(conf); - CONNECTION_INSTANCES.put(connectionKey, connection); - } else if (connection.isClosed()) { - ConnectionManager.deleteConnection(connectionKey, true); - connection = (ConnectionImplementation) ConnectionFactory.createConnection(conf); - CONNECTION_INSTANCES.put(connectionKey, connection); - } - connection.incCount(); - return connection; - } + static ClusterConnection getConnectionInternal(final Configuration conf) throws IOException { + return (ConnectionImplementation) ConnectionFactory.createConnection(conf); } /** @@ -159,7 +102,7 @@ final class ConnectionManager { static ClusterConnection createConnectionInternal(Configuration conf) throws IOException { UserProvider provider = UserProvider.instantiate(conf); - return createConnection(conf, false, null, provider.getCurrent()); + return (ClusterConnection) createConnection(conf, null, provider.getCurrent()); } /** @@ -185,7 +128,7 @@ final class ConnectionManager { public static HConnection createConnection(Configuration conf, ExecutorService pool) throws IOException { UserProvider provider = UserProvider.instantiate(conf); - return createConnection(conf, false, pool, provider.getCurrent()); + return createConnection(conf, pool, provider.getCurrent()); } /** @@ -210,7 +153,7 @@ final class ConnectionManager { */ public static HConnection createConnection(Configuration conf, User user) throws IOException { - return createConnection(conf, false, null, user); + return createConnection(conf, null, user); } /** @@ -236,78 +179,10 @@ final class ConnectionManager { */ public static HConnection createConnection(Configuration conf, ExecutorService pool, User user) throws IOException { - return createConnection(conf, false, pool, user); - } - - /** - * @deprecated instead use one of the {@link ConnectionFactory#createConnection()} methods. - */ - @Deprecated - static HConnection createConnection(final Configuration conf, final boolean managed) - throws IOException { - UserProvider provider = UserProvider.instantiate(conf); - return createConnection(conf, managed, null, provider.getCurrent()); - } - - /** - * @deprecated instead use one of the {@link ConnectionFactory#createConnection()} methods. - */ - @Deprecated - static ClusterConnection createConnection(final Configuration conf, final boolean managed, - final ExecutorService pool, final User user) - throws IOException { - return (ClusterConnection) ConnectionFactory.createConnection(conf, managed, pool, user); - } - - /** - * Cleanup a known stale connection. - * This will then close connection to the zookeeper ensemble and let go of all resources. - * - * @param connection - * @deprecated connection caching is going away. - */ - @Deprecated - public static void deleteStaleConnection(HConnection connection) { - deleteConnection(connection, true); - } - - /** - * @deprecated connection caching is going away. - */ - @Deprecated - static void deleteConnection(HConnection connection, boolean staleConnection) { - synchronized (CONNECTION_INSTANCES) { - for (Entry e: CONNECTION_INSTANCES.entrySet()) { - if (e.getValue() == connection) { - deleteConnection(e.getKey(), staleConnection); - break; - } - } - } + return (ClusterConnection) ConnectionFactory.createConnection(conf, pool, user); } /** - * @deprecated connection caching is going away. -˙ */ - @Deprecated - private static void deleteConnection(HConnectionKey connectionKey, boolean staleConnection) { - synchronized (CONNECTION_INSTANCES) { - ConnectionImplementation connection = CONNECTION_INSTANCES.get(connectionKey); - if (connection != null) { - connection.decCount(); - if (connection.isZeroReference() || staleConnection) { - CONNECTION_INSTANCES.remove(connectionKey); - connection.internalClose(); - } - } else { - LOG.error("Connection not found in the list, can't delete it "+ - "(connection key=" + connectionKey + "). May be the key was modified?", new Exception()); - } - } - } - - - /** * This convenience method invokes the given {@link HConnectable#connect} * implementation using a {@link HConnection} instance that lasts just for the * duration of the invocation. @@ -323,7 +198,7 @@ final class ConnectionManager { return null; } Configuration conf = connectable.conf; - HConnection connection = getConnection(conf); + HConnection connection = getConnectionInternal(conf); boolean connectSucceeded = false; try { T returnValue = connectable.connect(connection); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java index f12c33e..323915b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java @@ -147,9 +147,9 @@ public final class ConnectionUtils { * region re-lookups. */ static class MasterlessConnection extends ConnectionImplementation { - MasterlessConnection(Configuration conf, boolean managed, + MasterlessConnection(Configuration conf, ExecutorService pool, User user) throws IOException { - super(conf, managed, pool, user); + super(conf, pool, user); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnection.java index e4f05b0..10090f2 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnection.java @@ -79,7 +79,6 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). * @param tableName * @return an HTable to use for interactions with this table @@ -92,7 +91,6 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). * @param tableName * @return an HTable to use for interactions with this table @@ -105,7 +103,6 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). * @param tableName * @return an HTable to use for interactions with this table @@ -119,7 +116,6 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). * @param tableName * @param pool The thread pool to use for batch operations, null to use a default pool. @@ -133,7 +129,6 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). * @param tableName * @param pool The thread pool to use for batch operations, null to use a default pool. @@ -147,9 +142,8 @@ public interface HConnection extends Connection { * be created for each using thread. * This is a lightweight operation, pooling or caching of the returned HTableInterface * is neither required nor desired. - * Note that the HConnection needs to be unmanaged * (created with {@link ConnectionFactory#createConnection(Configuration)}). - * @param tableName + * @param tableName table to get interface for * @param pool The thread pool to use for batch operations, null to use a default pool. * @return an HTable to use for interactions with this table */ @@ -162,10 +156,6 @@ public interface HConnection extends Connection { * * This is a lightweight operation. Pooling or caching of the returned RegionLocator is neither * required nor desired. - * - * RegionLocator needs to be unmanaged - * (created with {@link ConnectionFactory#createConnection(Configuration)}). - * * @param tableName Name of the table who's region is to be examined * @return A RegionLocator instance */ @@ -176,7 +166,7 @@ public interface HConnection extends Connection { * Retrieve an Admin implementation to administer an HBase cluster. * The returned Admin is not guaranteed to be thread-safe. A new instance should be created for * each using thread. This is a lightweight operation. Pooling or caching of the returned - * Admin is not recommended. Note that HConnection needs to be unmanaged + * Admin is not recommended. * * @return an Admin instance for cluster administration */ diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java index 9671ea6..6d02649 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java @@ -266,8 +266,8 @@ public class TestClientNoCluster extends Configured implements Tool { final ClientService.BlockingInterface stub; ScanOpenNextThenExceptionThenRecoverConnection(Configuration conf, - boolean managed, ExecutorService pool) throws IOException { - super(conf, managed); + ExecutorService pool) throws IOException { + super(conf, pool, null); // Mock up my stub so open scanner returns a scanner id and then on next, we throw // exceptions for three times and then after that, we return no more to scan. this.stub = Mockito.mock(ClientService.BlockingInterface.class); @@ -297,9 +297,9 @@ public class TestClientNoCluster extends Configured implements Tool { extends ConnectionImplementation { final ClientService.BlockingInterface stub; - RegionServerStoppedOnScannerOpenConnection(Configuration conf, boolean managed, + RegionServerStoppedOnScannerOpenConnection(Configuration conf, ExecutorService pool, User user) throws IOException { - super(conf, managed); + super(conf, pool, user); // Mock up my stub so open scanner returns a scanner id and then on next, we throw // exceptions for three times and then after that, we return no more to scan. this.stub = Mockito.mock(ClientService.BlockingInterface.class); @@ -329,9 +329,9 @@ public class TestClientNoCluster extends Configured implements Tool { extends ConnectionImplementation { final ClientService.BlockingInterface stub; - RpcTimeoutConnection(Configuration conf, boolean managed, ExecutorService pool, User user) + RpcTimeoutConnection(Configuration conf, ExecutorService pool, User user) throws IOException { - super(conf, managed); + super(conf, pool, user); // Mock up my stub so an exists call -- which turns into a get -- throws an exception this.stub = Mockito.mock(ClientService.BlockingInterface.class); try { @@ -364,10 +364,10 @@ public class TestClientNoCluster extends Configured implements Tool { final AtomicLong sequenceids = new AtomicLong(0); private final Configuration conf; - ManyServersManyRegionsConnection(Configuration conf, boolean managed, + ManyServersManyRegionsConnection(Configuration conf, ExecutorService pool, User user) throws IOException { - super(conf, managed, pool, user); + super(conf, pool, user); int serverCount = conf.getInt("hbase.test.servers", 10); this.serversByClient = new HashMap(serverCount); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java index 52d22b7..df927ff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/client/CoprocessorHConnection.java @@ -43,7 +43,7 @@ public class CoprocessorHConnection extends ConnectionImplementation { private static final NonceGenerator NO_NONCE_GEN = new ConnectionManager.NoNonceGenerator(); /** - * Create an unmanaged {@link HConnection} based on the environment in which we are running the + * Create an {@link HConnection} based on the environment in which we are running the * coprocessor. The {@link HConnection} must be externally cleaned up (we bypass the usual HTable * cleanup mechanisms since we own everything). * @param env environment hosting the {@link HConnection} @@ -95,7 +95,7 @@ public class CoprocessorHConnection extends ConnectionImplementation { * @throws IOException if we cannot create the connection */ public CoprocessorHConnection(Configuration conf, HRegionServer server) throws IOException { - super(conf, false, null, UserProvider.instantiate(conf).getCurrent()); + super(conf, null, UserProvider.instantiate(conf).getCurrent()); this.server = server; this.serverName = server.getServerName(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/ServerResourceCheckerJUnitListener.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/ServerResourceCheckerJUnitListener.java index 4e01b5e..4b750e4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/ServerResourceCheckerJUnitListener.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/ServerResourceCheckerJUnitListener.java @@ -19,24 +19,9 @@ package org.apache.hadoop.hbase; -import org.apache.hadoop.hbase.ResourceChecker.Phase; -import org.apache.hadoop.hbase.client.HConnectionTestingUtility; - /** * Monitor the resources. use by the tests All resources in {@link ResourceCheckerJUnitListener} * plus the number of connection. */ public class ServerResourceCheckerJUnitListener extends ResourceCheckerJUnitListener { - - static class ConnectionCountResourceAnalyzer extends ResourceChecker.ResourceAnalyzer { - @Override - public int getVal(Phase phase) { - return HConnectionTestingUtility.getConnectionCount(); - } - } - - @Override - protected void addResourceAnalyzer(ResourceChecker rc) { - rc.addResourceAnalyzer(new ConnectionCountResourceAnalyzer()); - } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java index 0a534b0..364ab6e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java @@ -52,17 +52,9 @@ public class HConnectionTestingUtility { */ public static ClusterConnection getMockedConnection(final Configuration conf) throws ZooKeeperConnectionException { - HConnectionKey connectionKey = new HConnectionKey(conf); - synchronized (ConnectionManager.CONNECTION_INSTANCES) { - ConnectionImplementation connection = - ConnectionManager.CONNECTION_INSTANCES.get(connectionKey); - if (connection == null) { - connection = Mockito.mock(ConnectionImplementation.class); - Mockito.when(connection.getConfiguration()).thenReturn(conf); - ConnectionManager.CONNECTION_INSTANCES.put(connectionKey, connection); - } - return connection; - } + ConnectionImplementation connection = Mockito.mock(ConnectionImplementation.class); + Mockito.when(connection.getConfiguration()).thenReturn(conf); + return connection; } /** @@ -99,7 +91,6 @@ public class HConnectionTestingUtility { throws IOException { ConnectionImplementation c = Mockito.mock(ConnectionImplementation.class); Mockito.when(c.getConfiguration()).thenReturn(conf); - ConnectionManager.CONNECTION_INSTANCES.put(new HConnectionKey(conf), c); Mockito.doNothing().when(c).close(); // Make it so we return a particular location when asked. final HRegionLocation loc = new HRegionLocation(hri, sn); @@ -151,38 +142,15 @@ public class HConnectionTestingUtility { */ public static ClusterConnection getSpiedConnection(final Configuration conf) throws IOException { - HConnectionKey connectionKey = new HConnectionKey(conf); - synchronized (ConnectionManager.CONNECTION_INSTANCES) { - ConnectionImplementation connection = - ConnectionManager.CONNECTION_INSTANCES.get(connectionKey); - if (connection == null) { - connection = Mockito.spy(new ConnectionImplementation(conf, false)); - ConnectionManager.CONNECTION_INSTANCES.put(connectionKey, connection); - } - return connection; - } + ConnectionImplementation connection = + Mockito.spy(new ConnectionImplementation(conf, null, null)); + return connection; } public static ClusterConnection getSpiedClusterConnection(final Configuration conf) throws IOException { - HConnectionKey connectionKey = new HConnectionKey(conf); - synchronized (ConnectionManager.CONNECTION_INSTANCES) { - ConnectionImplementation connection = - ConnectionManager.CONNECTION_INSTANCES.get(connectionKey); - if (connection == null) { - connection = Mockito.spy(new ConnectionImplementation(conf, false)); - ConnectionManager.CONNECTION_INSTANCES.put(connectionKey, connection); - } - return connection; - } - } - - /** - * @return Count of extant connection instances - */ - public static int getConnectionCount() { - synchronized (ConnectionManager.CONNECTION_INSTANCES) { - return ConnectionManager.CONNECTION_INSTANCES.size(); - } + ConnectionImplementation connection = + Mockito.spy(new ConnectionImplementation(conf, null, null)); + return connection; } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java index a352c4e..0eec736 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java @@ -633,20 +633,6 @@ public class TestAdmin2 { } /** - * HBASE-4417 checkHBaseAvailable() doesn't close zk connections - */ - @Test (timeout=300000) - public void testCheckHBaseAvailableClosesConnection() throws Exception { - Configuration conf = TEST_UTIL.getConfiguration(); - - int initialCount = HConnectionTestingUtility.getConnectionCount(); - HBaseAdmin.checkHBaseAvailable(conf); - int finalCount = HConnectionTestingUtility.getConnectionCount(); - - Assert.assertEquals(initialCount, finalCount) ; - } - - /** * Check that we have an exception if the cluster is not there. */ @Test (timeout=300000) @@ -657,8 +643,6 @@ public class TestAdmin2 { conf.setInt(HConstants.ZOOKEEPER_CLIENT_PORT, conf.getInt(HConstants.ZOOKEEPER_CLIENT_PORT, 9999)+10); - int initialCount = HConnectionTestingUtility.getConnectionCount(); - long start = System.currentTimeMillis(); try { HBaseAdmin.checkHBaseAvailable(conf); @@ -670,10 +654,6 @@ public class TestAdmin2 { } long end = System.currentTimeMillis(); - int finalCount = HConnectionTestingUtility.getConnectionCount(); - - Assert.assertEquals(initialCount, finalCount) ; - LOG.info("It took "+(end-start)+" ms to find out that" + " HBase was not available"); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java index 0b08562..fadfacb 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java @@ -23,15 +23,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.net.SocketTimeoutException; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.concurrent.ExecutorService; import java.util.concurrent.SynchronousQueue; @@ -46,7 +43,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionLocation; @@ -73,7 +69,6 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.JVMClusterUtil; import org.apache.hadoop.hbase.util.ManualEnvironmentEdge; import org.apache.hadoop.hbase.util.Threads; -import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.jboss.netty.util.internal.DetectionUtil; import org.junit.AfterClass; import org.junit.Assert; @@ -141,11 +136,6 @@ public class TestHCM { TEST_UTIL.shutdownMiniCluster(); } - - private static int getHConnectionManagerCacheSize(){ - return HConnectionTestingUtility.getConnectionCount(); - } - @Test public void testClusterConnection() throws IOException { ThreadPoolExecutor otherPool = new ThreadPoolExecutor(1, 1, @@ -535,7 +525,6 @@ public class TestHCM { } finally { syncBlockingFilter.set(true); t.join(); - ConnectionManager.getConnection(c2).close(); TEST_UTIL.getHBaseAdmin().setBalancerRunning(previousBalance, true); } @@ -568,28 +557,6 @@ public class TestHCM { } } - @Test - public void abortingHConnectionRemovesItselfFromHCM() throws Exception { - // Save off current HConnections - Map oldHBaseInstances = - new HashMap(); - oldHBaseInstances.putAll(ConnectionManager.CONNECTION_INSTANCES); - - ConnectionManager.CONNECTION_INSTANCES.clear(); - - try { - HConnection connection = ConnectionManager.getConnection(TEST_UTIL.getConfiguration()); - connection.abort("test abortingHConnectionRemovesItselfFromHCM", new Exception( - "test abortingHConnectionRemovesItselfFromHCM")); - Assert.assertNotSame(connection, - ConnectionManager.getConnection(TEST_UTIL.getConfiguration())); - } finally { - // Put original HConnections back - ConnectionManager.CONNECTION_INSTANCES.clear(); - ConnectionManager.CONNECTION_INSTANCES.putAll(oldHBaseInstances); - } - } - /** * Test that when we delete a location using the first row of a region * that we really delete it. @@ -846,35 +813,6 @@ public class TestHCM { table.close(); } - /** - * Make sure that {@link Configuration} instances that are essentially the - * same map to the same {@link HConnection} instance. - */ - @Test - public void testConnectionSameness() throws Exception { - Connection previousConnection = null; - for (int i = 0; i < 2; i++) { - // set random key to differentiate the connection from previous ones - Configuration configuration = TEST_UTIL.getConfiguration(); - configuration.set("some_key", String.valueOf(_randy.nextInt())); - LOG.info("The hash code of the current configuration is: " - + configuration.hashCode()); - Connection currentConnection = ConnectionManager - .getConnection(configuration); - if (previousConnection != null) { - assertTrue( - "Did not get the same connection even though its key didn't change", - previousConnection == currentConnection); - } - previousConnection = currentConnection; - // change the configuration, so that it is no longer reachable from the - // client's perspective. However, since its part of the LRU doubly linked - // list, it will eventually get thrown out, at which time it should also - // close the corresponding {@link HConnection}. - configuration.set("other_key", String.valueOf(_randy.nextInt())); - } - } - @Test public void testClosing() throws Exception { Configuration configuration = @@ -911,13 +849,8 @@ public class TestHCM { // created from the same configuration, yet they are different assertTrue(c1 != c2); assertTrue(c1.getConfiguration() == c2.getConfiguration()); - // make sure these were not cached - Connection c3 = ConnectionManager.getConnection(configuration); - assertTrue(c1 != c3); - assertTrue(c2 != c3); } - /** * This test checks that one can connect to the cluster with only the * ZooKeeper quorum set. Other stuff like master address will be read @@ -933,7 +866,7 @@ public class TestHCM { TEST_UTIL.getConfiguration().get(HConstants.ZOOKEEPER_CLIENT_PORT)); // This should be enough to connect - HConnection conn = ConnectionManager.getConnection(c); + HConnection conn = ConnectionManager.createConnection(c); assertTrue( conn.isMasterRunning() ); conn.close(); } @@ -1130,85 +1063,6 @@ public class TestHCM { Math.abs(actual - expected) <= (0.01f * jitterBase)); } - /** - * Tests that a destroyed connection does not have a live zookeeper. - * Below is timing based. We put up a connection to a table and then close the connection while - * having a background thread running that is forcing close of the connection to try and - * provoke a close catastrophe; we are hoping for a car crash so we can see if we are leaking - * zk connections. - * @throws Exception - */ - @Ignore ("Flakey test: See HBASE-8996")@Test - public void testDeleteForZKConnLeak() throws Exception { - TEST_UTIL.createTable(TABLE_NAME4, FAM_NAM); - final Configuration config = HBaseConfiguration.create(TEST_UTIL.getConfiguration()); - config.setInt("zookeeper.recovery.retry", 1); - config.setInt("zookeeper.recovery.retry.intervalmill", 1000); - config.setInt("hbase.rpc.timeout", 2000); - config.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 1); - - ThreadPoolExecutor pool = new ThreadPoolExecutor(1, 10, - 5, TimeUnit.SECONDS, - new SynchronousQueue(), - Threads.newDaemonThreadFactory("test-hcm-delete")); - - pool.submit(new Runnable() { - @Override - public void run() { - while (!Thread.interrupted()) { - try { - HConnection conn = ConnectionManager.getConnection(config); - LOG.info("Connection " + conn); - ConnectionManager.deleteStaleConnection(conn); - LOG.info("Connection closed " + conn); - // TODO: This sleep time should be less than the time that it takes to open and close - // a table. Ideally we would do a few runs first to measure. For now this is - // timing based; hopefully we hit the bad condition. - Threads.sleep(10); - } catch (Exception e) { - } - } - } - }); - - // Use connection multiple times. - for (int i = 0; i < 30; i++) { - Connection c1 = null; - try { - c1 = ConnectionManager.getConnectionInternal(config); - LOG.info("HTable connection " + i + " " + c1); - Table table = c1.getTable(TABLE_NAME4, pool); - table.close(); - LOG.info("HTable connection " + i + " closed " + c1); - } catch (Exception e) { - LOG.info("We actually want this to happen!!!! So we can see if we are leaking zk", e); - } finally { - if (c1 != null) { - if (c1.isClosed()) { - // cannot use getZooKeeper as method instantiates watcher if null - Field zkwField = c1.getClass().getDeclaredField("keepAliveZookeeper"); - zkwField.setAccessible(true); - Object watcher = zkwField.get(c1); - - if (watcher != null) { - if (((ZooKeeperWatcher)watcher).getRecoverableZooKeeper().getState().isAlive()) { - // non-synchronized access to watcher; sleep and check again in case zk connection - // hasn't been cleaned up yet. - Thread.sleep(1000); - if (((ZooKeeperWatcher) watcher).getRecoverableZooKeeper().getState().isAlive()) { - pool.shutdownNow(); - fail("Live zookeeper in closed connection"); - } - } - } - } - c1.close(); - } - } - } - pool.shutdownNow(); - } - @Test(timeout = 60000) public void testConnectionRideOverClusterRestart() throws IOException, InterruptedException { Configuration config = new Configuration(TEST_UTIL.getConfiguration()); -- 1.9.5 (Apple Git-50.3)