HBASE-12029 Use Table and RegionLocator in HTable.getRegionLocations() (Solomon Duskis)

Conflicts:
	hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
	hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionSizeCalculator.java

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionManager.java

@@ -1004,7 +1004,7 @@ class ConnectionManager {
     public List<HRegionLocation> locateRegions(final TableName tableName,
         final boolean useCache, final boolean offlined) throws IOException {
       NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, this,
-          tableName, offlined);
+          tableName);
       final List<HRegionLocation> locations = new ArrayList<HRegionLocation>();
       for (HRegionInfo regionInfo : regions.keySet()) {
         RegionLocations list = locateRegion(tableName, regionInfo.getStartKey(), useCache, true);

hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java

@@ -25,6 +25,7 @@ import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.NavigableMap;
 import java.util.TreeMap;
 import java.util.concurrent.Callable;
@@ -614,12 +615,29 @@ public class HTable implements HTableInterface, RegionLocator {
    * This is mainly useful for the MapReduce integration.
    * @return A map of HRegionInfo with it's server address
    * @throws IOException if a remote or network exception occurs
-   * @deprecated This is no longer a public API
+   * @deprecated This is no longer a public API.  Use {@link #getAllRegionLocations()} instead.
    */
   @Deprecated
   public NavigableMap<HRegionInfo, ServerName> getRegionLocations() throws IOException {
     // TODO: Odd that this returns a Map of HRI to SN whereas getRegionLocator, singular, returns an HRegionLocation.
-    return MetaScanner.allTableRegions(getConfiguration(), this.connection, getName(), false);
+    return MetaScanner.allTableRegions(getConfiguration(), this.connection, getName());
+  }
+
+  /**
+   * Gets all the regions and their address for this table.
+   * <p>
+   * This is mainly useful for the MapReduce integration.
+   * @return A map of HRegionInfo with it's server address
+   * @throws IOException if a remote or network exception occurs
+   */
+  @Override
+  public List<HRegionLocation> getAllRegionLocations() throws IOException {
+    NavigableMap<HRegionInfo, ServerName> locations = getRegionLocations();
+    ArrayList<HRegionLocation> regions = new ArrayList<>(locations.size());
+    for (Entry<HRegionInfo, ServerName> entry : locations.entrySet()) {
+      regions.add(new HRegionLocation(entry.getKey(), entry.getValue()));
+    }
+    return regions;
   }
 
   /**

hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java

@@ -132,8 +132,10 @@ public class MetaScanner {
       final byte[] row, final int rowLimit, final TableName metaTableName)
     throws IOException {
 
+    boolean closeConnection = false;
     if (connection == null){
-      connection = ConnectionManager.getConnectionInternal(configuration);
+      connection = ConnectionFactory.createConnection(configuration);
+      closeConnection = true;
     }
 
     int rowUpperLimit = rowLimit > 0 ? rowLimit: Integer.MAX_VALUE;
@@ -217,7 +219,9 @@ public class MetaScanner {
           LOG.debug("Got exception in closing meta table", t);
         }
       }
-
+      if (closeConnection) {
+        connection.close();
+      }
     }
   }
 
@@ -269,6 +273,21 @@ public class MetaScanner {
     return regions;
   }
 
+  /**
+   * Lists all of the table regions currently in META.
+   * @param conf
+   * @param offlined True if we are to include offlined regions, false and we'll
+   * leave out offlined regions from returned list.
+   * @return Map of all user-space regions to servers
+   * @throws IOException
+   * @deprecated Use {@link #allTableRegions(Configuration, Connection, TableName)} instead
+   */
+  @Deprecated
+  public static NavigableMap<HRegionInfo, ServerName> allTableRegions(Configuration conf,
+      Connection connection, final TableName tableName, boolean offlined) throws IOException {
+    return allTableRegions(conf, connection, tableName);
+  }
+
   /**
    * Lists all of the table regions currently in META.
    * @param conf
@@ -278,8 +297,7 @@ public class MetaScanner {
    * @throws IOException
    */
   public static NavigableMap<HRegionInfo, ServerName> allTableRegions(Configuration conf,
-      Connection connection, final TableName tableName,
+      Connection connection, final TableName tableName) throws IOException {
-      final boolean offlined) throws IOException {
     final NavigableMap<HRegionInfo, ServerName> regions =
       new TreeMap<HRegionInfo, ServerName>();
     MetaScannerVisitor visitor = new TableMetaScannerVisitor(tableName) {

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionLocator.java

@@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.util.Pair;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.List;
 
 /**
  * Used to view region location information for a single HBase table.
@@ -57,6 +58,14 @@ public interface RegionLocator extends Closeable {
   public HRegionLocation getRegionLocation(final byte [] row, boolean reload)
     throws IOException;
 
+  /**
+   * Retrieves all of the regions associated with this table.
+   * @return a {@link List} of all regions associated with this table.
+   * @throws IOException if a remote or network exception occurs
+   */
+  public List<HRegionLocation> getAllRegionLocations()
+    throws IOException;
+
   /**
    * Gets the starting row key for every region in the currently open table.
    * <p>

hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RegionsResource.java

@@ -77,7 +77,7 @@ public class RegionsResource extends ResourceBase {
       TableName tableName = TableName.valueOf(tableResource.getName());
       TableInfoModel model = new TableInfoModel(tableName.getNameAsString());
       Map<HRegionInfo,ServerName> regions = MetaScanner.allTableRegions(
-        servlet.getConfiguration(), null, tableName, false);
+        servlet.getConfiguration(), null, tableName);
       for (Map.Entry<HRegionInfo,ServerName> e: regions.entrySet()) {
         HRegionInfo hri = e.getKey();
         ServerName addr = e.getValue();

hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableInputFormatBase.java

@@ -157,7 +157,8 @@ public abstract class MultiTableInputFormatBase extends
         byte[] startRow = scan.getStartRow();
         byte[] stopRow = scan.getStopRow();
 
-        RegionSizeCalculator sizeCalculator = new RegionSizeCalculator((HTable) table);
+        RegionSizeCalculator sizeCalculator = new RegionSizeCalculator(
+            regionLocator, conn.getAdmin());
 
         for (int i = 0; i < keys.getFirst().length; i++) {
           if (!includeRegionInSplit(keys.getFirst()[i], keys.getSecond()[i])) {
@@ -184,7 +185,7 @@ public abstract class MultiTableInputFormatBase extends
 
             long regionSize = sizeCalculator.getRegionSize(regionInfo.getRegionName());
             TableSplit split =
-                new TableSplit(table.getName(),
+                new TableSplit(regionLocator.getName(),
                     scan, splitStart, splitStop, regionHostname, regionSize);
 
             splits.add(split);

hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormat.java

@@ -109,6 +109,8 @@ implements Configurable {
     this.conf = configuration;
     TableName tableName = TableName.valueOf(conf.get(INPUT_TABLE));
     try {
+      // TODO: Replace setHTable() with initializeTable() once we have 
+      //       a clean method of closing a connection.
       setHTable(new HTable(new Configuration(conf), tableName));
     } catch (Exception e) {
       LOG.error(StringUtils.stringifyException(e));

hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java

@@ -35,6 +35,7 @@ import org.apache.hadoop.hbase.classification.InterfaceStability;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.RegionLocator;
@@ -286,14 +287,27 @@ extends InputFormat<ImmutableBytesWritable, Result> {
    * Allows subclasses to set the {@link HTable}.
    *
    * @param table  The table to get the data from.
+   * @throws IOExceptfion 
    * @deprecated Use {@link #initializeTable(Connection, TableName)} instead.
    */
   @Deprecated
-  protected void setHTable(HTable table) {
+  protected void setHTable(HTable table) throws IOException {
     this.table = table;
     this.regionLocator = table;
   }
 
+  /**
+   * Allows subclasses to initalize the table information.
+   *
+   * @param connection  The {@link Connection} to the HBase cluster.
+   * @param tableName  The {@link TableName} of the table to process. 
+   * @throws IOExceptfion 
+   */
+  protected void initializeTable(Connection connection, TableName tableName) throws IOException {
+    this.table = connection.getTable(tableName);
+    this.regionLocator = connection.getRegionLocator(tableName);
+  }
+
   /**
    * Gets the scan defining the actual details like columns etc.
    *

hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSizeCalculator.java

@@ -17,29 +17,31 @@
  */
 package org.apache.hadoop.hbase.util;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
-import org.apache.hadoop.hbase.classification.InterfaceStability;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.ClusterStatus;
-import org.apache.hadoop.hbase.HRegionInfo;
-import org.apache.hadoop.hbase.RegionLoad;
-import org.apache.hadoop.hbase.ServerLoad;
-import org.apache.hadoop.hbase.ServerName;
-import org.apache.hadoop.hbase.client.Admin;
-import org.apache.hadoop.hbase.client.HBaseAdmin;
-import org.apache.hadoop.hbase.client.HTable;
-
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ClusterStatus;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLoad;
+import org.apache.hadoop.hbase.ServerLoad;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.HBaseAdmin;
+import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.RegionLocator;
+
 /**
  * Computes size of each region for given table and given column families.
  * The value is used by MapReduce for better scheduling.
@@ -59,57 +61,65 @@ public class RegionSizeCalculator {
 
   /**
    * Computes size of each region for table and given column families.
-   * */
+   * 
+   * @deprecated Use {@link #RegionSizeCalculator(RegionLocator, Admin)} instead.
+   */
+  @Deprecated
   public RegionSizeCalculator(HTable table) throws IOException {
-    this(table, new HBaseAdmin(table.getConfiguration()));
+    HBaseAdmin admin = new HBaseAdmin(table.getConfiguration());
-  }
-
-  /** ctor for unit testing */
-  RegionSizeCalculator (HTable table, Admin admin) throws IOException {
-
     try {
-      if (!enabled(table.getConfiguration())) {
+      init(table, admin);
-        LOG.info("Region size calculation disabled.");
-        return;
-      }
-
-      LOG.info("Calculating region sizes for table \"" + new String(table.getTableName()) + "\".");
-
-      //get regions for table
-      Set<HRegionInfo> tableRegionInfos = table.getRegionLocations().keySet();
-      Set<byte[]> tableRegions = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
-      for (HRegionInfo regionInfo : tableRegionInfos) {
-        tableRegions.add(regionInfo.getRegionName());
-      }
-
-      ClusterStatus clusterStatus = admin.getClusterStatus();
-      Collection<ServerName> servers = clusterStatus.getServers();
-      final long megaByte = 1024L * 1024L;
-
-      //iterate all cluster regions, filter regions from our table and compute their size
-      for (ServerName serverName: servers) {
-        ServerLoad serverLoad = clusterStatus.getLoad(serverName);
-
-        for (RegionLoad regionLoad: serverLoad.getRegionsLoad().values()) {
-          byte[] regionId = regionLoad.getName();
-
-          if (tableRegions.contains(regionId)) {
-
-            long regionSizeBytes = regionLoad.getStorefileSizeMB() * megaByte;
-            sizeMap.put(regionId, regionSizeBytes);
-
-            if (LOG.isDebugEnabled()) {
-              LOG.debug("Region " + regionLoad.getNameAsString() + " has size " + regionSizeBytes);
-            }
-          }
-        }
-      }
-      LOG.debug("Region sizes calculated");
-
     } finally {
       admin.close();
     }
+  }
 
+  /**
+   * Computes size of each region for table and given column families.
+   * */
+  public RegionSizeCalculator(RegionLocator regionLocator, Admin admin) throws IOException {
+    init(regionLocator, admin);
+  }
+
+  private void init(RegionLocator regionLocator, Admin admin)
+      throws IOException {
+    if (!enabled(admin.getConfiguration())) {
+      LOG.info("Region size calculation disabled.");
+      return;
+    }
+
+    LOG.info("Calculating region sizes for table \"" + regionLocator.getName() + "\".");
+
+    //get regions for table
+    List<HRegionLocation> tableRegionInfos = regionLocator.getAllRegionLocations();
+    Set<byte[]> tableRegions = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
+    for (HRegionLocation regionInfo : tableRegionInfos) {
+      tableRegions.add(regionInfo.getRegionInfo().getRegionName());
+    }
+
+    ClusterStatus clusterStatus = admin.getClusterStatus();
+    Collection<ServerName> servers = clusterStatus.getServers();
+    final long megaByte = 1024L * 1024L;
+
+    //iterate all cluster regions, filter regions from our table and compute their size
+    for (ServerName serverName: servers) {
+      ServerLoad serverLoad = clusterStatus.getLoad(serverName);
+
+      for (RegionLoad regionLoad: serverLoad.getRegionsLoad().values()) {
+        byte[] regionId = regionLoad.getName();
+
+        if (tableRegions.contains(regionId)) {
+
+          long regionSizeBytes = regionLoad.getStorefileSizeMB() * megaByte;
+          sizeMap.put(regionId, regionSizeBytes);
+
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("Region " + regionLoad.getNameAsString() + " has size " + regionSizeBytes);
+          }
+        }
+      }
+    }
+    LOG.debug("Region sizes calculated");
   }
 
   boolean enabled(Configuration configuration) {

hbase-server/src/test/java/org/apache/hadoop/hbase/TestMultiVersions.java

@@ -210,8 +210,7 @@ public class TestMultiVersions {
     NavigableMap<HRegionInfo, ServerName> locations = table.getRegionLocations();
     assertEquals(2, locations.size());
     int index = 0;
-    for (Map.Entry<HRegionInfo, ServerName> e: locations.entrySet()) {
+    for (HRegionInfo hri: locations.keySet()) {
-      HRegionInfo hri = e.getKey();
       if (index == 0) {
         assertTrue(Bytes.equals(HConstants.EMPTY_START_ROW, hri.getStartKey()));
         assertTrue(Bytes.equals(hri.getEndKey(), splitRows[0]));

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java

@@ -189,7 +189,7 @@ public class TestMetaScanner {
          while(!isStopped()) {
            try {
             NavigableMap<HRegionInfo, ServerName> regions =
-                MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), null, TABLENAME, false);
+                MetaScanner.allTableRegions(TEST_UTIL.getConfiguration(), null, TABLENAME);
 
             LOG.info("-------");
             byte[] lastEndKey = HConstants.EMPTY_START_ROW;

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java

@@ -253,7 +253,7 @@ public class TestEndToEndSplitTransaction {
         Random random = new Random();
         for (int i= 0; i< 5; i++) {
           NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, null,
-              tableName, false);
+              tableName);
           if (regions.size() == 0) {
             continue;
           }
@@ -326,7 +326,7 @@ public class TestEndToEndSplitTransaction {
 
       //MetaScanner.allTableRegions()
       NavigableMap<HRegionInfo, ServerName> regions = MetaScanner.allTableRegions(conf, null,
-          tableName, false);
+          tableName);
       verifyTableRegions(regions.keySet());
 
       //MetaScanner.listAllRegions()

hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionSizeCalculator.java

@@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.util;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterStatus;
 import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.RegionLoad;
 import org.apache.hadoop.hbase.ServerLoad;
 import org.apache.hadoop.hbase.ServerName;
@@ -27,6 +28,8 @@ import org.apache.hadoop.hbase.SmallTests;
 import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionLocator;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Mockito;
@@ -35,7 +38,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-import java.util.NavigableMap;
 import java.util.TreeMap;
 
 import static org.junit.Assert.assertEquals;
@@ -51,7 +53,7 @@ public class TestRegionSizeCalculator {
   @Test
   public void testSimpleTestCase() throws Exception {
 
-    HTable table = mockTable("region1", "region2", "region3");
+    RegionLocator regionLocator = mockRegionLocator("region1", "region2", "region3");
 
     Admin admin = mockAdmin(
       mockServer(
@@ -64,7 +66,7 @@ public class TestRegionSizeCalculator {
       )
     );
 
-    RegionSizeCalculator calculator = new RegionSizeCalculator(table, admin);
+    RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
 
     assertEquals(123 * megabyte, calculator.getRegionSize("region1".getBytes()));
     assertEquals(54321 * megabyte, calculator.getRegionSize("region2".getBytes()));
@@ -83,7 +85,7 @@ public class TestRegionSizeCalculator {
   @Test
   public void testLargeRegion() throws Exception {
 
-    HTable table = mockTable("largeRegion");
+    RegionLocator regionLocator = mockRegionLocator("largeRegion");
 
     Admin admin = mockAdmin(
       mockServer(
@@ -91,7 +93,7 @@ public class TestRegionSizeCalculator {
       )
     );
 
-    RegionSizeCalculator calculator = new RegionSizeCalculator(table, admin);
+    RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
 
     assertEquals(((long) Integer.MAX_VALUE) * megabyte, calculator.getRegionSize("largeRegion".getBytes()));
   }
@@ -100,7 +102,7 @@ public class TestRegionSizeCalculator {
   @Test
   public void testDisabled() throws Exception {
     String regionName = "cz.goout:/index.html";
-    HTable table = mockTable(regionName);
+    RegionLocator table = mockRegionLocator(regionName);
 
     Admin admin = mockAdmin(
       mockServer(
@@ -123,29 +125,29 @@ public class TestRegionSizeCalculator {
   /**
    * Makes some table with given region names.
    * */
-  private HTable mockTable(String... regionNames) throws IOException {
+  private RegionLocator mockRegionLocator(String... regionNames) throws IOException {
-    HTable mockedTable = Mockito.mock(HTable.class);
+    RegionLocator mockedTable = Mockito.mock(RegionLocator.class);
-    when(mockedTable.getConfiguration()).thenReturn(configuration);
+    when(mockedTable.getName()).thenReturn(TableName.valueOf("sizeTestTable"));
-    when(mockedTable.getTableName()).thenReturn("sizeTestTable".getBytes());
+    List<HRegionLocation> regionLocations = new ArrayList<>();
-    NavigableMap<HRegionInfo, ServerName> regionLocations = new TreeMap<HRegionInfo, ServerName>();
+    when(mockedTable.getAllRegionLocations()).thenReturn(regionLocations);
-    when(mockedTable.getRegionLocations()).thenReturn(regionLocations);
 
     for (String regionName : regionNames) {
       HRegionInfo info = Mockito.mock(HRegionInfo.class);
       when(info.getRegionName()).thenReturn(regionName.getBytes());
-      regionLocations.put(info, null);//we are not interested in values
+      regionLocations.add(new HRegionLocation(info, null));//we are not interested in values
     }
 
     return mockedTable;
   }
 
   /**
-   * Creates mock returing ClusterStatus info about given servers.
+   * Creates mock returning ClusterStatus info about given servers.
   */
   private Admin mockAdmin(ServerLoad... servers) throws Exception {
     //get clusterstatus
-    Admin mockAdmin = Mockito.mock(HBaseAdmin.class);
+    Admin mockAdmin = Mockito.mock(Admin.class);
     ClusterStatus clusterStatus = mockCluster(servers);
+    when(mockAdmin.getConfiguration()).thenReturn(configuration);
     when(mockAdmin.getClusterStatus()).thenReturn(clusterStatus);
     return mockAdmin;
   }