Index: src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java (revision 992199) +++ src/test/java/org/apache/hadoop/hbase/HBaseClusterTestCase.java (working copy) @@ -172,7 +172,7 @@ } super.tearDown(); try { - HConnectionManager.deleteConnectionInfo(conf, true); + HConnectionManager.deleteConnection(conf, true); if (this.cluster != null) { try { this.cluster.shutdown(); Index: src/test/java/org/apache/hadoop/hbase/util/DisabledTestMetaUtils.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/util/DisabledTestMetaUtils.java (revision 992199) +++ src/test/java/org/apache/hadoop/hbase/util/DisabledTestMetaUtils.java (working copy) @@ -50,7 +50,7 @@ utils.deleteColumn(editTable, Bytes.toBytes(oldColumn)); utils.shutdown(); // Delete again so we go get it all fresh. - HConnectionManager.deleteConnectionInfo(conf, false); + HConnectionManager.deleteConnection(conf, false); // Now assert columns were added and deleted. this.cluster = new MiniHBaseCluster(this.conf, 1); // Now assert columns were added and deleted. Index: src/test/java/org/apache/hadoop/hbase/client/TestHCM.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/client/TestHCM.java (revision 992199) +++ src/test/java/org/apache/hadoop/hbase/client/TestHCM.java (working copy) @@ -19,11 +19,25 @@ */ package org.apache.hadoop.hbase.client; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.Set; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionLocation; +import org.apache.hadoop.hbase.ZooKeeperConnectionException; import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.mortbay.log.Log; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNotNull; @@ -32,7 +46,6 @@ * This class is for testing HCM features */ public class TestHCM { - private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); private static final byte[] TABLE_NAME = Bytes.toBytes("test"); private static final byte[] FAM_NAM = Bytes.toBytes("f"); @@ -43,7 +56,86 @@ TEST_UTIL.startMiniCluster(1); } + @AfterClass public static void tearDownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + /** + * @throws InterruptedException + * @throws IllegalAccessException + * @throws NoSuchFieldException + * @throws ZooKeeperConnectionException + * @throws IllegalArgumentException + * @throws SecurityException + * @see https://issues.apache.org/jira/browse/HBASE-2925 + */ + @Test public void testManyNewConnectionsDoesnotOOME() + throws SecurityException, IllegalArgumentException, + ZooKeeperConnectionException, NoSuchFieldException, IllegalAccessException, + InterruptedException { + createNewConfigurations(); + } + + private static Random _randy = new Random(); + + public static void createNewConfigurations() throws SecurityException, + IllegalArgumentException, NoSuchFieldException, + IllegalAccessException, InterruptedException, ZooKeeperConnectionException { + HConnection last = null; + for (int i = 0; i <= (HConnectionManager.MAX_CACHED_HBASE_INSTANCES * 2); i++) { + // set random key to differentiate the connection from previous ones + Configuration configuration = HBaseConfiguration.create(); + configuration.set("somekey", String.valueOf(_randy.nextInt())); + System.out.println("Hash Code: " + configuration.hashCode()); + HConnection connection = + HConnectionManager.getConnection(configuration); + if (last != null) { + if (last == connection) { + System.out.println("!! Got same connection for once !!"); + } + } + // change the configuration once, and the cached connection is lost forever: + // the hashtable holding the cache won't be able to find its own keys + // to remove them, so the LRU strategy does not work. + configuration.set("someotherkey", String.valueOf(_randy.nextInt())); + last = connection; + Log.info("Cache Size: " + + getHConnectionManagerCacheSize() + ", Valid Keys: " + + getValidKeyCount()); + Thread.sleep(100); + } + Assert.assertEquals(HConnectionManager.MAX_CACHED_HBASE_INSTANCES, + getHConnectionManagerCacheSize()); + Assert.assertEquals(HConnectionManager.MAX_CACHED_HBASE_INSTANCES, + getValidKeyCount()); + } + + private static int getHConnectionManagerCacheSize() + throws SecurityException, NoSuchFieldException, + IllegalArgumentException, IllegalAccessException { + Field cacheField = + HConnectionManager.class.getDeclaredField("HBASE_INSTANCES"); + cacheField.setAccessible(true); + Map cache = (Map) cacheField.get(null); + return cache.size(); + } + + private static int getValidKeyCount() throws SecurityException, + NoSuchFieldException, IllegalArgumentException, + IllegalAccessException { + Field cacheField = + HConnectionManager.class.getDeclaredField("HBASE_INSTANCES"); + cacheField.setAccessible(true); + Map cache = (Map) cacheField.get(null); + List keys = new ArrayList(cache.keySet()); + Set values = new HashSet(); + for (Object key : keys) { + values.add(cache.get(key)); + } + return values.size(); + } + + /** * Test that when we delete a location using the first row of a region * that we really delete it. * @throws Exception @@ -62,5 +154,4 @@ HRegionLocation rl = conn.getCachedLocation(TABLE_NAME, ROW); assertNull("What is this location?? " + rl, rl); } -} - +} \ No newline at end of file Index: src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java (working copy) @@ -19,7 +19,6 @@ */ package org.apache.hadoop.hbase; -import java.util.Iterator; import java.util.Map.Entry; import org.apache.commons.logging.Log; @@ -87,57 +86,4 @@ } return conf; } - - /** - * Returns the hash code value for this HBaseConfiguration. The hash code of a - * HBaseConfiguration is defined by the xor of the hash codes of its entries. - * - * @see Configuration#iterator() How the entries are obtained. - */ - @Override - @Deprecated - public int hashCode() { - return hashCode(this); - } - - /** - * Returns the hash code value for this HBaseConfiguration. The hash code of a - * Configuration is defined by the xor of the hash codes of its entries. - * - * @see Configuration#iterator() How the entries are obtained. - */ - public static int hashCode(Configuration conf) { - int hash = 0; - - Iterator> propertyIterator = conf.iterator(); - while (propertyIterator.hasNext()) { - hash ^= propertyIterator.next().hashCode(); - } - return hash; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (!(obj instanceof HBaseConfiguration)) - return false; - HBaseConfiguration otherConf = (HBaseConfiguration) obj; - if (size() != otherConf.size()) { - return false; - } - Iterator> propertyIterator = this.iterator(); - while (propertyIterator.hasNext()) { - Entry entry = propertyIterator.next(); - String key = entry.getKey(); - String value = entry.getValue(); - if (!value.equals(otherConf.getRaw(key))) { - return false; - } - } - - return true; - } } Index: src/main/java/org/apache/hadoop/hbase/util/HMerge.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/util/HMerge.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/util/HMerge.java (working copy) @@ -106,7 +106,7 @@ HConnection connection = HConnectionManager.getConnection(conf); masterIsRunning = connection.isMasterRunning(); } - HConnectionManager.deleteConnectionInfo(conf, false); + HConnectionManager.deleteConnection(conf, false); if (Bytes.equals(tableName, HConstants.META_TABLE_NAME)) { if (masterIsRunning) { throw new IllegalStateException( Index: src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (working copy) @@ -42,7 +42,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.DoNotRetryIOException; -import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HRegionLocation; @@ -70,9 +69,7 @@ import org.apache.zookeeper.KeeperException; /** - * A non-instantiable class that manages connections to multiple tables in - * multiple HBase instances. - * + * A non-instantiable class that manages connections to tables. * Used by {@link HTable} and {@link HBaseAdmin} */ @SuppressWarnings("serial") @@ -87,44 +84,43 @@ }); } - /* - * Not instantiable. - */ - protected HConnectionManager() { - super(); - } + static final int MAX_CACHED_HBASE_INSTANCES = 31; - private static final int MAX_CACHED_HBASE_INSTANCES=31; - // A LRU Map of master HBaseConfiguration -> connection information for that - // instance. The objects it contains are mutable and hence require - // synchronized access to them. We set instances to 31. The zk default max - // connections is 30 so should run into zk issues before hit this value of 31. - private static - final Map HBASE_INSTANCES = - new LinkedHashMap + // A LRU Map of Configuration hashcode -> TableServers. We set instances to 31. + // The zk default max connections to the ensemble from the one client is 30 so + // should run into zk issues before hit this value of 31. + private static final Map HBASE_INSTANCES = + new LinkedHashMap ((int) (MAX_CACHED_HBASE_INSTANCES/0.75F)+1, 0.75F, true) { @Override - protected boolean removeEldestEntry(Map.Entry eldest) { + protected boolean removeEldestEntry(Map.Entry eldest) { return size() > MAX_CACHED_HBASE_INSTANCES; } }; + /* + * Non-instantiable. + */ + protected HConnectionManager() { + super(); + } + /** - * Get the connection object for the instance specified by the configuration - * If no current connection exists, create a new connection for that instance + * Get the connection that goes with the passed conf configuration. + * If no current connection exists, method creates a new connection for the + * passed conf instance. * @param conf configuration - * @return HConnection object for the instance specified by the configuration + * @return HConnection object for conf * @throws ZooKeeperConnectionException */ public static HConnection getConnection(Configuration conf) throws ZooKeeperConnectionException { TableServers connection; - Integer key = HBaseConfiguration.hashCode(conf); synchronized (HBASE_INSTANCES) { - connection = HBASE_INSTANCES.get(key); + connection = HBASE_INSTANCES.get(conf); if (connection == null) { connection = new TableServers(conf); - HBASE_INSTANCES.put(key, connection); + HBASE_INSTANCES.put(conf, connection); } } return connection; @@ -135,11 +131,9 @@ * @param conf configuration * @param stopProxy stop the proxy as well */ - public static void deleteConnectionInfo(Configuration conf, - boolean stopProxy) { + public static void deleteConnection(Configuration conf, boolean stopProxy) { synchronized (HBASE_INSTANCES) { - Integer key = HBaseConfiguration.hashCode(conf); - TableServers t = HBASE_INSTANCES.remove(key); + TableServers t = HBASE_INSTANCES.remove(conf); if (t != null) { t.close(stopProxy); } @@ -168,7 +162,8 @@ * @throws ZooKeeperConnectionException */ static int getCachedRegionCount(Configuration conf, - byte[] tableName) throws ZooKeeperConnectionException { + byte[] tableName) + throws ZooKeeperConnectionException { TableServers connection = (TableServers)getConnection(conf); return connection.getNumberOfCachedRegionLocations(tableName); } @@ -185,7 +180,7 @@ return connection.isRegionCached(tableName, row); } - /* Encapsulates finding the servers for an HBase instance */ + /* Encapsulates connection to zookeeper and regionservers.*/ static class TableServers implements ServerConnection, Abortable { static final Log LOG = LogFactory.getLog(TableServers.class); private final Class serverInterfaceClass; @@ -208,7 +203,7 @@ private final Object metaRegionLock = new Object(); private final Object userRegionLock = new Object(); - private volatile Configuration conf; + private final Configuration conf; // Known region HServerAddress.toString() -> HRegionInterface private final Map servers = Index: src/main/java/org/apache/hadoop/hbase/client/HTableFactory.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HTableFactory.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/client/HTableFactory.java (working copy) @@ -29,7 +29,6 @@ * @since 0.21.0 */ public class HTableFactory implements HTableInterfaceFactory { - @Override public HTableInterface createHTableInterface(Configuration config, byte[] tableName) { @@ -47,9 +46,5 @@ } catch (IOException ioe) { throw new RuntimeException(ioe); } - } - - - -} +} \ No newline at end of file Index: src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java (working copy) @@ -408,7 +408,7 @@ } } // Delete cached information to prevent clients from using old locations - HConnectionManager.deleteConnectionInfo(conf, false); + HConnectionManager.deleteConnection(conf, false); LOG.info("Deleted " + Bytes.toString(tableName)); } Index: src/main/java/org/apache/hadoop/hbase/client/HTable.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HTable.java (revision 992199) +++ src/main/java/org/apache/hadoop/hbase/client/HTable.java (working copy) @@ -82,9 +82,11 @@ /** * Creates an object to access a HBase table. - * - * @param tableName Name of the table. + * Internally it creates a new instance of {@link Configuration} and a new + * client to zookeeper as well as other resources. Use only when being quick + * and dirty. * @throws IOException if a remote or network exception occurs + * @see #HTable(Configuration, String) */ public HTable(final String tableName) throws IOException { @@ -93,9 +95,12 @@ /** * Creates an object to access a HBase table. - * + * Internally it creates a new instance of {@link Configuration} and a new + * client to zookeeper as well as other resources. Use only when being quick + * and dirty. * @param tableName Name of the table. * @throws IOException if a remote or network exception occurs + * @see #HTable(Configuration, String) */ public HTable(final byte [] tableName) throws IOException { @@ -104,7 +109,8 @@ /** * Creates an object to access a HBase table. - * + * Shares zookeeper connection and other resources with other HTable instances + * created with same conf. Recommended. * @param conf Configuration object to use. * @param tableName Name of the table. * @throws IOException if a remote or network exception occurs @@ -117,7 +123,8 @@ /** * Creates an object to access a HBase table. - * + * Shares zookeeper connection and other resources with other HTable instances + * created with same conf. Recommended. * @param conf Configuration object to use. * @param tableName Name of the table. * @throws IOException if a remote or network exception occurs