HBASE-5507 ThriftServerRunner.HbaseHandler.getRegionInfo() and getTableRegions() do not use ByteBuffer correctly (Scott Chen)

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

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java

@@ -19,6 +19,8 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import static org.apache.hadoop.hbase.thrift.ThriftServerRunner.HBaseHandler.toBytes;
+
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.List;
@@ -119,8 +121,9 @@ public class HRegionThriftServer extends Thread {
                                                 long timestamp,
       Map<ByteBuffer, ByteBuffer> attributes) throws IOError {
       try {
-        byte [] row = rowb.array();
-        HTable table = getTable(tableName.array());
+        byte[] row = toBytes(rowb);
+
+        HTable table = getTable(toBytes(tableName));
         HRegionLocation location = table.getRegionLocation(row, false);
         byte[] regionName = location.getRegionInfo().getRegionName();
 
@@ -130,11 +133,9 @@ public class HRegionThriftServer extends Thread {
           Result result = rs.get(regionName, get);
           return ThriftUtilities.rowResultFromHBase(result);
         }
-        ByteBuffer[] columnArr = columns.toArray(
-                                   new ByteBuffer[columns.size()]);
         Get get = new Get(row);
-        for(ByteBuffer column : columnArr) {
-          byte [][] famAndQf = KeyValue.parseColumn(column.array());
+        for(ByteBuffer column : columns) {
+          byte [][] famAndQf = KeyValue.parseColumn(toBytes(column));
           if (famAndQf.length == 1) {
             get.addFamily(famAndQf[0]);
           } else {

src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java

@@ -585,7 +585,7 @@ public class ThriftServerRunner implements Runnable {
     throws IOError {
       try{
         List<HRegionInfo> hris =
-            this.getHBaseAdmin().getTableRegions(tableName.array());
+            this.getHBaseAdmin().getTableRegions(toBytes(tableName));
         List<TRegionInfo> regions = new ArrayList<TRegionInfo>();
 
         if (hris != null) {
@@ -606,6 +606,18 @@ public class ThriftServerRunner implements Runnable {
       }
     }
 
+    /**
+     * Convert ByteBuffer to byte array. Note that this cannot be replaced by
+     * Bytes.toBytes().
+     */
+    public static byte[] toBytes(ByteBuffer bb) {
+      byte[] result = new byte[bb.remaining()];
+      // Make a duplicate so the position doesn't change
+      ByteBuffer dup = bb.duplicate();
+      dup.get(result, 0, result.length);
+      return result;
+    }
+
     @Deprecated
     @Override
     public List<TCell> get(
@@ -1337,12 +1349,13 @@ public class ThriftServerRunner implements Runnable {
     public TRegionInfo getRegionInfo(ByteBuffer searchRow) throws IOError {
       try {
         HTable table = getTable(HConstants.META_TABLE_NAME);
+        byte[] row = toBytes(searchRow);
         Result startRowResult = table.getRowOrBefore(
-          searchRow.array(), HConstants.CATALOG_FAMILY);
+          row, HConstants.CATALOG_FAMILY);
 
         if (startRowResult == null) {
           throw new IOException("Cannot find row in .META., row="
-                                + Bytes.toStringBinary(searchRow.array()));
+                                + Bytes.toStringBinary(row));
         }
 
         // find region start and end keys
@@ -1351,7 +1364,7 @@ public class ThriftServerRunner implements Runnable {
         if (value == null || value.length == 0) {
           throw new IOException("HRegionInfo REGIONINFO was null or " +
                                 " empty in Meta for row="
-                                + Bytes.toStringBinary(searchRow.array()));
+                                + Bytes.toStringBinary(row));
         }
         HRegionInfo regionInfo = Writables.getHRegionInfo(value);
         TRegionInfo region = new TRegionInfo();

src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java

@@ -429,6 +429,11 @@ public class TestThriftServer {
   public void doTestGetTableRegions() throws Exception {
     ThriftServerRunner.HBaseHandler handler =
       new ThriftServerRunner.HBaseHandler(UTIL.getConfiguration());
+    doTestGetTableRegions(handler);
+  }
+
+  public static void doTestGetTableRegions(Hbase.Iface handler)
+      throws Exception {
     handler.createTable(tableAname, getColumnDescriptors());
     int regionCount = handler.getTableRegions(tableAname).size();
     assertEquals("empty table should have only 1 region, " +

src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java

@@ -87,15 +87,6 @@ public class TestThriftServerCmdLine {
             continue;
           }
           for (boolean specifyCompact : new boolean[] {false, true}) {
-            // TODO: We observed that when
-            // (specifyFramed, specifyCompact) == (true, false).
-            // The method getRegionInfo() gets a corrupted parameter. This may
-            // be a thrift bug that needs further investigation. In this test we
-            // temporarily exclude these cases to avoid test failures.
-            if ((specifyFramed == true || implType.isAlwaysFramed ) &&
-                specifyCompact == false) {
-              continue;
-            }
             parameters.add(new Object[]{implType, new Boolean(specifyFramed),
                 new Boolean(specifyBindIP), new Boolean(specifyCompact)});
           }
@@ -217,6 +208,7 @@ public class TestThriftServerCmdLine {
       Hbase.Client client = new Hbase.Client(prot);
       TestThriftServer.doTestTableCreateDrop(client);
       TestThriftServer.doTestGetRegionInfo(client);
+      TestThriftServer.doTestGetTableRegions(client);
     } finally {
       sock.close();
     }