From d42ecb89bd06e06f73a1b48a7b02d23b5e50cdd1 Mon Sep 17 00:00:00 2001 From: larsh Date: Sun, 28 Jul 2013 06:03:31 +0000 Subject: [PATCH] HBASE-8698 potential thread creation in MetaScanner.metaScan git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1507766 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop/hbase/client/HBaseAdmin.java | 8 +++-- .../hbase/client/HConnectionManager.java | 12 +++---- .../apache/hadoop/hbase/client/HTable.java | 5 +-- .../hadoop/hbase/client/MetaScanner.java | 36 +++++++++---------- .../hadoop/hbase/master/CatalogJanitor.java | 2 +- .../hadoop/hbase/rest/RegionsResource.java | 2 +- .../hadoop/hbase/client/TestMetaScanner.java | 4 +-- .../hbase/master/TestRestartCluster.java | 4 +-- .../TestEndToEndSplitTransaction.java | 7 ++-- 9 files changed, 40 insertions(+), 40 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index fffc2134915..73e058fa022 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -152,6 +152,7 @@ public class HBaseAdmin implements Abortable, Closeable { // want to wait a long time. private final int retryLongerMultiplier; private boolean aborted; + private boolean cleanupConnectionOnClose = false; // close the connection in close() /** * Constructor. @@ -164,6 +165,7 @@ public class HBaseAdmin implements Abortable, Closeable { // Will not leak connections, as the new implementation of the constructor // does not throw exceptions anymore. this(HConnectionManager.getConnection(new Configuration(c))); + this.cleanupConnectionOnClose = true; } /** @@ -446,7 +448,7 @@ public class HBaseAdmin implements Abortable, Closeable { return true; } }; - MetaScanner.metaScan(conf, visitor, desc.getName()); + MetaScanner.metaScan(conf, connection, visitor, desc.getName()); if (actualRegCount.get() != numRegs) { if (tries == this.numRetries * this.retryLongerMultiplier - 1) { throw new RegionOfflineException("Only " + actualRegCount.get() + @@ -1863,7 +1865,7 @@ public class HBaseAdmin implements Abortable, Closeable { } }; - MetaScanner.metaScan(conf, visitor); + MetaScanner.metaScan(conf, connection, visitor, null); pair = result.get(); } return pair; @@ -2038,7 +2040,7 @@ public class HBaseAdmin implements Abortable, Closeable { @Override public void close() throws IOException { - if (this.connection != null) { + if (cleanupConnectionOnClose && this.connection != null) { this.connection.close(); } } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java index e0ccd154150..b3b6ae4600c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java @@ -677,7 +677,7 @@ public class HConnectionManager { return true; } }; - MetaScanner.metaScan(conf, visitor, tableName); + MetaScanner.metaScan(conf, this, visitor, tableName); return available.get() && (regionCount.get() > 0); } @@ -717,7 +717,7 @@ public class HConnectionManager { return true; } }; - MetaScanner.metaScan(conf, visitor, tableName); + MetaScanner.metaScan(conf, this, visitor, tableName); // +1 needs to be added so that the empty start row is also taken into account return available.get() && (regionCount.get() == splitKeys.length + 1); } @@ -746,8 +746,8 @@ public class HConnectionManager { @Override public List locateRegions(final byte[] tableName, final boolean useCache, final boolean offlined) throws IOException { - NavigableMap regions = MetaScanner.allTableRegions(conf, tableName, - offlined); + NavigableMap regions = MetaScanner.allTableRegions(conf, this, + tableName, offlined); final List locations = new ArrayList(); for (HRegionInfo regionInfo : regions.keySet()) { locations.add(locateRegion(tableName, regionInfo.getStartKey(), useCache, true)); @@ -838,8 +838,8 @@ public class HConnectionManager { }; try { // pre-fetch certain number of regions info at region cache. - MetaScanner.metaScan(conf, visitor, tableName, row, - this.prefetchRegionLimit); + MetaScanner.metaScan(conf, this, visitor, tableName, row, + this.prefetchRegionLimit, HConstants.META_TABLE_NAME); } catch (IOException e) { LOG.warn("Encountered problems when prefetch META table: ", e); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java index e4e4dd57773..25e67a4d68e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java @@ -227,9 +227,6 @@ public class HTable implements HTableInterface { */ public HTable(final byte[] tableName, final HConnection connection, final ExecutorService pool) throws IOException { - if (pool == null || pool.isShutdown()) { - throw new IllegalArgumentException("Pool is null or shut down."); - } if (connection == null || connection.isClosed()) { throw new IllegalArgumentException("Connection is null or closed."); } @@ -501,7 +498,7 @@ public class HTable implements HTableInterface { */ public NavigableMap getRegionLocations() throws IOException { // TODO: Odd that this returns a Map of HRI to SN whereas getRegionLocation, singular, returns an HRegionLocation. - return MetaScanner.allTableRegions(getConfiguration(), getTableName(), false); + return MetaScanner.allTableRegions(getConfiguration(), this.connection, getTableName(), false); } /** diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java index d62e44f6651..1f1189cd017 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java @@ -61,7 +61,7 @@ public class MetaScanner { public static void metaScan(Configuration configuration, MetaScannerVisitor visitor) throws IOException { - metaScan(configuration, visitor, null); + metaScan(configuration, visitor, null, null, Integer.MAX_VALUE); } /** @@ -69,15 +69,17 @@ public class MetaScanner { * name to locate meta regions. * * @param configuration config + * @param connection connection to use internally (null to use a new instance) * @param visitor visitor object * @param userTableName User table name in meta table to start scan at. Pass * null if not interested in a particular table. * @throws IOException e */ - public static void metaScan(Configuration configuration, + public static void metaScan(Configuration configuration, HConnection connection, MetaScannerVisitor visitor, byte [] userTableName) throws IOException { - metaScan(configuration, visitor, userTableName, null, Integer.MAX_VALUE); + metaScan(configuration, connection, visitor, userTableName, null, Integer.MAX_VALUE, + HConstants.META_TABLE_NAME); } /** @@ -99,7 +101,7 @@ public class MetaScanner { MetaScannerVisitor visitor, byte [] userTableName, byte[] row, int rowLimit) throws IOException { - metaScan(configuration, visitor, userTableName, row, rowLimit, + metaScan(configuration, null, visitor, userTableName, row, rowLimit, HConstants.META_TABLE_NAME); } @@ -109,6 +111,7 @@ public class MetaScanner { * rowLimit of rows. * * @param configuration HBase configuration. + * @param connection connection to use internally (null to use a new instance) * @param visitor Visitor object. Closes the visitor before returning. * @param tableName User table name in meta table to start scan at. Pass * null if not interested in a particular table. @@ -119,12 +122,17 @@ public class MetaScanner { * @param metaTableName Meta table to scan, root or meta. * @throws IOException e */ - public static void metaScan(Configuration configuration, + public static void metaScan(Configuration configuration, HConnection connection, final MetaScannerVisitor visitor, final byte[] tableName, final byte[] row, final int rowLimit, final byte[] metaTableName) throws IOException { int rowUpperLimit = rowLimit > 0 ? rowLimit: Integer.MAX_VALUE; - HTable metaTable = new HTable(configuration, HConstants.META_TABLE_NAME); + HTable metaTable; + if (connection == null) { + metaTable = new HTable(configuration, HConstants.META_TABLE_NAME, null); + } else { + metaTable = new HTable(HConstants.META_TABLE_NAME, connection, null); + } // Calculate startrow for scan. byte[] startRow; ResultScanner scanner = null; @@ -215,17 +223,8 @@ public class MetaScanner { } /** - * Lists all of the regions currently in META. - * @param conf - * @return List of all user-space regions. - * @throws IOException - */ - public static List listAllRegions(Configuration conf) - throws IOException { - return listAllRegions(conf, true); - } - - /** + * Used in tests. + * * Lists all of the regions currently in META. * @param conf * @param offlined True if we are to include offlined regions, false and we'll @@ -268,6 +267,7 @@ public class MetaScanner { * @throws IOException */ public static NavigableMap allTableRegions(Configuration conf, + HConnection connection, final byte [] tablename, final boolean offlined) throws IOException { final NavigableMap regions = new TreeMap(); @@ -280,7 +280,7 @@ public class MetaScanner { return true; } }; - metaScan(conf, visitor, tablename); + metaScan(conf, connection, visitor, tablename); return regions; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java index 7381f8bf489..f9d07343b50 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java @@ -162,7 +162,7 @@ public class CatalogJanitor extends Chore { // Run full scan of .META. catalog table passing in our custom visitor with // the start row - MetaScanner.metaScan(server.getConfiguration(), visitor, tableName); + MetaScanner.metaScan(server.getConfiguration(), null, visitor, tableName); return new Triple, Map>( count.get(), mergedRegions, splitParents); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RegionsResource.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RegionsResource.java index ce9d463772f..b6ffde62cc0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RegionsResource.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RegionsResource.java @@ -77,7 +77,7 @@ public class RegionsResource extends ResourceBase { String tableName = tableResource.getName(); TableInfoModel model = new TableInfoModel(tableName); Map regions = MetaScanner.allTableRegions( - servlet.getConfiguration(), Bytes.toBytes(tableName), false); + servlet.getConfiguration(), null, Bytes.toBytes(tableName), false); for (Map.Entry e: regions.entrySet()) { HRegionInfo hri = e.getKey(); ServerName addr = e.getValue(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java index 61bdfff6bb3..af6619b1472 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java @@ -85,7 +85,7 @@ public class TestMetaScanner { doReturn(true).when(visitor).processRow((Result)anyObject()); // Scanning the entire table should give us three rows - MetaScanner.metaScan(conf, visitor, TABLENAME); + MetaScanner.metaScan(conf, null, visitor, TABLENAME); verify(visitor, times(3)).processRow((Result)anyObject()); // Scanning the table with a specified empty start row should also @@ -188,7 +188,7 @@ public class TestMetaScanner { while(!isStopped()) { try { NavigableMap regions = - MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), TABLENAME, false); + MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), null, TABLENAME, false); LOG.info("-------"); byte[] lastEndKey = HConstants.EMPTY_START_ROW; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java index 0ebb78ff3ef..3af775ff9a5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java @@ -102,7 +102,7 @@ public class TestRestartCluster { } List allRegions = - MetaScanner.listAllRegions(UTIL.getConfiguration()); + MetaScanner.listAllRegions(UTIL.getConfiguration(), true); assertEquals(3, allRegions.size()); LOG.info("\n\nShutting down cluster"); @@ -118,7 +118,7 @@ public class TestRestartCluster { // Otherwise we're reusing an HConnection that has gone stale because // the shutdown of the cluster also called shut of the connection. allRegions = MetaScanner. - listAllRegions(new Configuration(UTIL.getConfiguration())); + listAllRegions(new Configuration(UTIL.getConfiguration()), true); assertEquals(3, allRegions.size()); LOG.info("\n\nWaiting for tables to be available"); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java index d8f58390b78..adb82cee119 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java @@ -222,7 +222,8 @@ public class TestEndToEndSplitTransaction { try { Random random = new Random(); for (int i=0; i< 5; i++) { - NavigableMap regions = MetaScanner.allTableRegions(conf, tableName, false); + NavigableMap regions = MetaScanner.allTableRegions(conf, null, + tableName, false); if (regions.size() == 0) { continue; } @@ -294,8 +295,8 @@ public class TestEndToEndSplitTransaction { void verifyRegionsUsingMetaScanner() throws Exception { //MetaScanner.allTableRegions() - NavigableMap regions = MetaScanner.allTableRegions(conf, tableName, - false); + NavigableMap regions = MetaScanner.allTableRegions(conf, null, + tableName, false); verifyTableRegions(regions.keySet()); //MetaScanner.listAllRegions()