HBASE-679 Regionserver addresses are still not right in the new tables page

Passed all regression tests and PerformanceEvaluation running with multiple region servers. table.jsp now displays the correct information

HRegionInfo:
- removed getTableNameFromRegionName and parseMetaRegionRow we have the information in the meta table, just use it.

HServerInfo:
- I had originally made some changes here but removed them. The only remaining changes are javadoc

MetaScanner:
- build region name using ZEROES instead of NINES. When you scan you need a row name that sorts before the first row rather than after the last row.
- scan using COLUMN_FAMILY_ARRAY instead of COL_REGIONINFO_ARRAY. This way you also get the server name and start code
- change api for MetaScannerVisitor so that processRow only gets the RowResult. If you have the RowResult you have everything you need.

HConnectionManager:
- change listTables' MetaScannerVisitor to conform to new processRow api

HTable:
- change getStartKeys' MetaScannerVisitor to conform to new processRow api
- getRegionsInfo: use new processRow api, and get the server address out of the RowResult, rather than relying on the one that is cached in HConnectionManager

ScannerCallable:
- make constructor public, add javadoc

HMaster
- change createTable to get the HRegionInfo out of the RowResult and compare table name from HRegionInfo with that in the HRegionInfo of the table being created, instead of parsing the table name out of the row

TestHTable
- modify test's MetaScannerVisitor to conform to the new processRow api

TestHRegionInfo
- remove testParse as parse method no longer exists

table.jsp
- change catch of IOException to catch Exception and print stack trace. At least you'll be able to see why the server crashes if it does.


git-svn-id: https://svn.apache.org/repos/asf/hadoop/hbase/trunk@678208 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -194,6 +194,7 @@ Trunk (unreleased changes)
    HBASE-42    Set region split size on table creation (Andrew Purtell via Stack)
    HBASE-43    Add a read-only attribute to columns (Andrew Purtell via Stack)
    HBASE-424   Should be able to enable/disable .META. table
+   HBASE-679   Regionserver addresses are still not right in the new tables page
    
   IMPROVEMENTS
    HBASE-559   MR example job to count table rows

src/java/org/apache/hadoop/hbase/HRegionInfo.java

@@ -22,8 +22,6 @@ package org.apache.hadoop.hbase;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
 
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.JenkinsHash;
@@ -57,62 +55,6 @@ public class HRegionInfo extends VersionedWritable implements WritableComparable
   public static final HRegionInfo FIRST_META_REGIONINFO =
     new HRegionInfo(1L, HTableDescriptor.META_TABLEDESC);
 
-  /**
-   * Extracts table name prefix from a metaregion row name.
-   * @param regionName A metaregion row name.
-   * @return The table prefix of a region name.
-   */
-  public static byte [] getTableNameFromRegionName(final byte [] regionName) {
-    return parseMetaRegionRow(regionName).get(0);
-  }
-
-  /**
-   * Parses passed metaregion row into its constituent parts.
-   * Presumes region names are ASCII characters only.
-   * @param regionName A metaregion row name.
-   * @return A list where first element is the tablename, second the row
-   * portion, and the third the id.
-   */
-  public static List<byte []> parseMetaRegionRow(final byte [] regionName) {
-    int offset = -1;
-    for (int i = 0; i < regionName.length; i++) {
-      if (regionName[i] == DELIMITER) {
-        offset = i;
-        break;
-      }
-    }
-    if (offset == -1) {
-      throw new IllegalArgumentException(Bytes.toString(regionName) +
-        " does not contain '" + DELIMITER + "' character");
-    }
-    byte [] tableName = new byte[offset];
-    System.arraycopy(regionName, 0, tableName, 0, offset);
-    // Now move in from the tail till we hit DELIMITER to find the id
-    offset = -1;
-    for (int i = regionName.length - 1; i > tableName.length; i--) {
-      if (regionName[i] == DELIMITER) {
-        offset = i;
-        break;
-      }
-    }
-    if (offset == -1) {
-      throw new IllegalArgumentException(Bytes.toString(regionName) +
-          " does not have parseable tail");
-    }
-    byte [] row = new byte[offset - (tableName.length + 1)];
-    System.arraycopy(regionName, tableName.length + 1, row, 0,
-      offset - (tableName.length + 1));
-    byte [] id = new byte[regionName.length - (offset + 1)];
-    System.arraycopy(regionName, offset + 1, id, 0,
-      regionName.length - (offset + 1));
-    // Now make up an array to hold the three parse pieces.
-    List<byte []> result = new ArrayList<byte []>(3);
-    result.add(tableName);
-    result.add(row);
-    result.add(id);
-    return result;
-  }
-
   private byte [] endKey = HConstants.EMPTY_BYTE_ARRAY;
   private boolean offLine = false;
   private long regionId = -1;
@@ -396,6 +338,8 @@ public class HRegionInfo extends VersionedWritable implements WritableComparable
     return this.hashCode;
   }
 
+  /** @return the object version number */
+  @Override
   public byte getVersion() {
     return VERSION;
   }
@@ -404,9 +348,8 @@ public class HRegionInfo extends VersionedWritable implements WritableComparable
   // Writable
   //
 
-  /**
+  /** {@inheritDoc} */
-   * {@inheritDoc}
+  @Override
-   */
   public void write(DataOutput out) throws IOException {
     super.write(out);
     Bytes.writeByteArray(out, endKey);
@@ -419,9 +362,8 @@ public class HRegionInfo extends VersionedWritable implements WritableComparable
     out.writeInt(hashCode);
   }
   
-  /**
+  /** {@inheritDoc} */
-   * {@inheritDoc}
+  @Override
-   */
   public void readFields(DataInput in) throws IOException {
     super.readFields(in);
     this.endKey = Bytes.readByteArray(in);

src/java/org/apache/hadoop/hbase/HServerInfo.java

@@ -122,11 +122,13 @@ public class HServerInfo implements WritableComparable {
     + ", load: (" + this.load.toString() + ")";
   }
 
+  /** {@inheritDoc} */
   @Override
   public boolean equals(Object obj) {
     return compareTo(obj) == 0;
   }
 
+  /** {@inheritDoc} */
   @Override
   public int hashCode() {
     int result = this.serverAddress.hashCode();
@@ -137,6 +139,8 @@ public class HServerInfo implements WritableComparable {
 
 
   // Writable
+  
+  /** {@inheritDoc} */
   public void readFields(DataInput in) throws IOException {
     this.serverAddress.readFields(in);
     this.startCode = in.readLong();
@@ -144,6 +148,7 @@ public class HServerInfo implements WritableComparable {
     this.infoPort = in.readInt();
   }
 
+  /** {@inheritDoc} */
   public void write(DataOutput out) throws IOException {
     this.serverAddress.write(out);
     out.writeLong(this.startCode);
@@ -151,6 +156,7 @@ public class HServerInfo implements WritableComparable {
     out.writeInt(this.infoPort);
   }
 
+  /** {@inheritDoc} */
   public int compareTo(Object o) {
     HServerInfo that = (HServerInfo)o;
     int result = getServerAddress().compareTo(that.getServerAddress());

src/java/org/apache/hadoop/hbase/client/HConnectionManager.java

@@ -285,10 +285,9 @@ public class HConnectionManager implements HConstants {
       MetaScannerVisitor visitor = new MetaScannerVisitor() {
 
         /** {@inheritDoc} */
-        public boolean processRow(
+        public boolean processRow(RowResult rowResult) throws IOException {
-            @SuppressWarnings("unused") RowResult rowResult,
+          HRegionInfo info = Writables.getHRegionInfo(
-            @SuppressWarnings("unused") HRegionLocation regionLocation,
+              rowResult.get(COL_REGIONINFO));
-            HRegionInfo info) {
 
           // Only examine the rows where the startKey is zero length
           if (info.getStartKey().length == 0) {

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

@@ -42,6 +42,7 @@ import org.apache.hadoop.hbase.io.BatchUpdate;
 import org.apache.hadoop.hbase.io.Cell;
 import org.apache.hadoop.hbase.io.RowResult;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.Writables;
 import org.apache.hadoop.io.Text;
 
 /**
@@ -258,14 +259,10 @@ public class HTable {
     final List<byte[]> keyList = new ArrayList<byte[]>();
 
     MetaScannerVisitor visitor = new MetaScannerVisitor() {
-      @SuppressWarnings("unused")
+      public boolean processRow(RowResult rowResult) throws IOException {
-      public boolean processRow(@SuppressWarnings("unused") RowResult rowResult,
+        HRegionInfo info = Writables.getHRegionInfo(
-          @SuppressWarnings("unused") HRegionLocation regionLocation,
+            rowResult.get(HConstants.COL_REGIONINFO));
-          HRegionInfo info)
+
-      throws IOException {
-        if (!(Bytes.equals(info.getTableDesc().getName(), getTableName()))) {
-          return false;
-        }
         if (!(info.isOffline() || info.isSplit())) {
           keyList.add(info.getStartKey());
         }
@@ -288,15 +285,23 @@ public class HTable {
       new TreeMap<HRegionInfo, HServerAddress>();
 
     MetaScannerVisitor visitor = new MetaScannerVisitor() {
-      public boolean processRow(@SuppressWarnings("unused") RowResult rowResult,
+      public boolean processRow(RowResult rowResult) throws IOException {
-          HRegionLocation regionLocation, HRegionInfo hri) {
+        HRegionInfo info = Writables.getHRegionInfo(
+            rowResult.get(HConstants.COL_REGIONINFO));
         
-        HRegionInfo info = new UnmodifyableHRegionInfo(hri); 
         if (!(Bytes.equals(info.getTableDesc().getName(), getTableName()))) {
           return false;
         }
+
+        HServerAddress server = new HServerAddress();
+        Cell c = rowResult.get(HConstants.COL_SERVER);
+        if (c != null && c.getValue() != null && c.getValue().length > 0) {
+          String address = Bytes.toString(c.getValue());
+          server = new HServerAddress(address);
+        }
+        
         if (!(info.isOffline() || info.isSplit())) {
-          regionMap.put(info, regionLocation.getServerAddress());
+          regionMap.put(new UnmodifyableHRegionInfo(info), server);
         }
         return true;
       }

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

@@ -1,15 +1,12 @@
 package org.apache.hadoop.hbase.client;
 
 import java.io.IOException;
-import java.util.List;
 
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
-import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.io.RowResult;
 import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.hadoop.hbase.util.Writables;
 
 /**
  * Scanner class that contains the <code>.META.</code> table scanning logic 
@@ -45,30 +42,24 @@ class MetaScanner implements HConstants {
       MetaScannerVisitor visitor, byte[] tableName)
   throws IOException {
     HConnection connection = HConnectionManager.getConnection(configuration);
-    boolean toContinue = true;
+    byte [] startRow = tableName == null || tableName.length == 0 ?
-    byte [] startRow = Bytes.equals(tableName, EMPTY_START_ROW)? tableName:
+        HConstants.EMPTY_START_ROW : 
-      HRegionInfo.createRegionName(tableName, null, NINES);
+          HRegionInfo.createRegionName(tableName, null, ZEROES);
       
     // Scan over each meta region
     do {
       ScannerCallable callable = new ScannerCallable(connection,
-        META_TABLE_NAME, COL_REGIONINFO_ARRAY, tableName, LATEST_TIMESTAMP,
+        META_TABLE_NAME, COLUMN_FAMILY_ARRAY, startRow, LATEST_TIMESTAMP, null);
-        null);
-      try {
       // Open scanner
       connection.getRegionServerWithRetries(callable);
-        while (toContinue) {
+      try {
-          RowResult rowResult = connection.getRegionServerWithRetries(callable);
+        RowResult r = null;
-          if (rowResult == null || rowResult.size() == 0) {
+        do {
+          r = connection.getRegionServerWithRetries(callable);
+          if (r == null || r.size() == 0) {
             break;
           }
-          HRegionInfo info = Writables.getHRegionInfo(rowResult
+        } while(visitor.processRow(r));
-              .get(COL_REGIONINFO));
-          List<byte []> parse = HRegionInfo.parseMetaRegionRow(info.getRegionName());
-          HRegionLocation regionLocation =
-            connection.locateRegion(parse.get(0), parse.get(1));
-          toContinue = visitor.processRow(rowResult, regionLocation, info);
-        }
         // Advance the startRow to the end key of the current region
         startRow = callable.getHRegionInfo().getEndKey();
       } finally {
@@ -89,12 +80,9 @@ class MetaScanner implements HConstants {
      * unnecessary for some reason.
      * 
      * @param rowResult
-     * @param regionLocation
-     * @param info
      * @return A boolean to know if it should continue to loop in the region
      * @throws IOException
      */
-    public boolean processRow(RowResult rowResult,
+    public boolean processRow(RowResult rowResult) throws IOException;
-        HRegionLocation regionLocation, HRegionInfo info) throws IOException;
   }
 }

src/java/org/apache/hadoop/hbase/client/ScannerCallable.java

@@ -39,7 +39,15 @@ public class ScannerCallable extends ServerCallable<RowResult> {
   private final long timestamp;
   private final RowFilterInterface filter;
 
-  protected ScannerCallable (HConnection connection, byte [] tableName, byte [][] columns,
+  /**
+   * @param connection
+   * @param tableName
+   * @param columns
+   * @param startRow
+   * @param timestamp
+   * @param filter
+   */
+  public ScannerCallable (HConnection connection, byte [] tableName, byte [][] columns,
       byte [] startRow, long timestamp, RowFilterInterface filter) {
     super(connection, tableName, startRow);
     this.columns = columns;

src/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -627,14 +627,10 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
       tableName, LATEST_TIMESTAMP, null);
     try {
       RowResult data = srvr.next(scannerid);
-          
-      // Test data and that the row for the data is for our table. If table
-      // does not exist, scanner will return row after where our table would
-      // be inserted if it exists so look for exact match on table name.            
       if (data != null && data.size() > 0) {
-        byte [] tn = HRegionInfo.getTableNameFromRegionName(data.getRow());
+        HRegionInfo info = Writables.getHRegionInfo(data.get(COL_REGIONINFO));
-        if (Bytes.equals(tn, tableName)) {
+        if (Bytes.equals(info.getTableDesc().getName(), tableName)) {
-          // Then a region for this table already exists. Ergo table exists.
+          // A region for this table already exists. Ergo table exists.
           throw new TableExistsException(Bytes.toString(tableName));
         }
       }

src/test/org/apache/hadoop/hbase/client/TestHTable.java

@@ -33,9 +33,9 @@ import org.apache.hadoop.hbase.io.BatchUpdate;
 import org.apache.hadoop.hbase.io.Cell;
 import org.apache.hadoop.hbase.io.RowResult;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.Writables;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.hbase.HRegionInfo;
-import org.apache.hadoop.hbase.HRegionLocation;
 
 /**
  * Tests HTable
@@ -137,7 +137,7 @@ public class TestHTable extends HBaseClusterTestCase implements HConstants {
 
     try {
       // make a modifiable descriptor
-      HTableDescriptor desc = new HTableDescriptor(a.getMetadata());
+      HTableDescriptor desc = new HTableDescriptor(a.getTableDescriptor());
       // offline the table
       admin.disableTable(tableAname);
       // add a user attribute to HTD
@@ -152,12 +152,13 @@ public class TestHTable extends HBaseClusterTestCase implements HConstants {
 
       // Use a metascanner to avoid client API caching (HConnection has a
       // metadata cache)
-      MetaScanner.MetaScannerVisitor visitor = new MetaScanner.MetaScannerVisitor() {
+      MetaScanner.MetaScannerVisitor visitor =
-          public boolean processRow(
+        new MetaScanner.MetaScannerVisitor() {
-              @SuppressWarnings("unused") RowResult rowResult,
+          public boolean processRow(RowResult rowResult) throws IOException {
-              HRegionLocation regionLocation,
+            HRegionInfo info = Writables.getHRegionInfo(
-              HRegionInfo info) {
+                rowResult.get(HConstants.COL_REGIONINFO));
-            LOG.info("visiting " + regionLocation.toString());
+
+            LOG.info("visiting " + info.toString());
             HTableDescriptor desc = info.getTableDesc();
             if (Bytes.compareTo(desc.getName(), tableAname) == 0) {
               // check HTD attribute

src/test/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java

@@ -19,25 +19,11 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
-import java.util.List;
-
 import org.apache.hadoop.hbase.HBaseTestCase;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.util.Bytes;
 
 public class TestHRegionInfo extends HBaseTestCase {
-  public void testParse() throws Exception {
-    String tableName = getName();
-    String row = getName();
-    long id = 12345;
-    List<byte []> parse =
-      HRegionInfo.parseMetaRegionRow(Bytes.toBytes(tableName + "," + row +
-        "," + Long.toString(id)));
-    assertEquals(Bytes.toString(parse.get(0)), tableName);
-    assertEquals(Bytes.toString(parse.get(1)), row);
-    assertEquals(Long.parseLong(Bytes.toString(parse.get(2))), id);
-  }
-  
   public void testCreateHRegionInfoName() throws Exception {
     String tableName = "tablename";
     final byte [] tn = Bytes.toBytes(tableName);

src/webapps/master/table.jsp

@@ -70,7 +70,9 @@
 </table>
 <%    } 
     }
-    catch(IOException ioe) { } 
+    catch(Exception ex) {
+      ex.printStackTrace();
+    } 
   }%>
 </body>
 </html>