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

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();
     }
   }

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<HRegionLocation> locateRegions(final byte[] tableName, final boolean useCache,
         final boolean offlined) throws IOException {
-      NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, tableName,
+      NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, this,
-        offlined);
+          tableName, offlined);
       final List<HRegionLocation> locations = new ArrayList<HRegionLocation>();
       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,
+        MetaScanner.metaScan(conf, this, visitor, tableName, row,
-            this.prefetchRegionLimit);
+            this.prefetchRegionLimit, HConstants.META_TABLE_NAME);
       } catch (IOException e) {
         LOG.warn("Encountered problems when prefetch META table: ", e);
       }

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<HRegionInfo, ServerName> 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);
   }
 
   /**

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 {
    * <code>rowLimit</code> 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.
+   * Used in tests.
-   * @param conf
+   *
-   * @return List of all user-space regions.
-   * @throws IOException
-   */
-  public static List<HRegionInfo> listAllRegions(Configuration conf)
-  throws IOException {
-    return listAllRegions(conf, true);
-  }
-
-  /**
    * 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<HRegionInfo, ServerName> allTableRegions(Configuration conf,
+      HConnection connection,
       final byte [] tablename, final boolean offlined) throws IOException {
     final NavigableMap<HRegionInfo, ServerName> regions =
       new TreeMap<HRegionInfo, ServerName>();
@@ -280,7 +280,7 @@ public class MetaScanner {
         return true;
       }
     };
-    metaScan(conf, visitor, tablename);
+    metaScan(conf, connection, visitor, tablename);
     return regions;
   }
 

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<Integer, Map<HRegionInfo, Result>, Map<HRegionInfo, Result>>(
         count.get(), mergedRegions, splitParents);

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<HRegionInfo,ServerName> regions = MetaScanner.allTableRegions(
-        servlet.getConfiguration(), Bytes.toBytes(tableName), false);
+        servlet.getConfiguration(), null, Bytes.toBytes(tableName), false);
       for (Map.Entry<HRegionInfo,ServerName> e: regions.entrySet()) {
         HRegionInfo hri = e.getKey();
         ServerName addr = e.getValue();

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<HRegionInfo, ServerName> regions =
-                MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), TABLENAME, false);
+                MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), null, TABLENAME, false);
 
             LOG.info("-------");
             byte[] lastEndKey = HConstants.EMPTY_START_ROW;

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java

@@ -102,7 +102,7 @@ public class TestRestartCluster {
     }
 
     List<HRegionInfo> 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");

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<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, tableName, false);
+          NavigableMap<HRegionInfo, ServerName> 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<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, tableName,
+      NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, null,
-          false);
+          tableName, false);
       verifyTableRegions(regions.keySet());
 
       //MetaScanner.listAllRegions()