HBASE-557 HTable.getRow() should receive RowResult objects

-Updated HRegionInterface, HRegionServer, HRegion, HStore to provide RowResults as the return of getRow methods
-Updated HTable to expect RowResult objects
-Updated ThriftServer to expect RowResults
-Cleaned up HConnectionManager's interaction with region servers

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

CHANGES.txt

@@ -18,6 +18,7 @@ Hbase Change Log
   IMPROVEMENTS
    HBASE-469   Streamline HStore startup and compactions
    HBASE-544   Purge startUpdate from internal code and test cases
+   HBASE-557   HTable.getRow() should receive RowResult objects
 
 Release 0.1.0
 

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

@@ -24,22 +24,16 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
-import java.util.SortedMap;
-import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.ipc.HbaseRPC;
 import org.apache.hadoop.hbase.util.Writables;
-import org.apache.hadoop.hbase.io.HbaseMapWritable;
 import org.apache.hadoop.io.Text;
-import org.apache.hadoop.io.Writable;
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.hbase.ipc.HMasterInterface;
 import org.apache.hadoop.hbase.util.SoftSortedMap;
-import org.apache.hadoop.hbase.HStoreKey;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HBaseConfiguration;
@@ -354,21 +348,6 @@ public class HConnectionManager implements HConstants {
       }
     }
 
-    /**
-      * Convenience method for turning a MapWritable into the underlying
-      * SortedMap we all know and love.
-      */
-    private SortedMap<Text, Cell> sortedMapFromMapWritable(
-      HbaseMapWritable writable) {
-      SortedMap<Text, Cell> results = new TreeMap<Text, Cell>();
-      for (Map.Entry<Writable, Writable> e: writable.entrySet()) {
-        HStoreKey key = (HStoreKey) e.getKey();
-        results.put(key.getColumn(), (Cell)e.getValue());
-      }
-      
-      return results;
-    }
-
     /**
       * Search one of the meta tables (-ROOT- or .META.) for the HRegionLocation
       * info that contains the table and row we're seeking.
@@ -412,7 +391,7 @@ public class HConnectionManager implements HConstants {
             getHRegionConnection(metaLocation.getServerAddress());
 
           // query the root region for the location of the meta region
-          HbaseMapWritable regionInfoRow = server.getClosestRowBefore(
+          RowResult regionInfoRow = server.getClosestRowBefore(
             metaLocation.getRegionInfo().getRegionName(), metaKey);
 
           if (regionInfoRow == null) {
@@ -420,11 +399,7 @@ public class HConnectionManager implements HConstants {
               "' does not exist.");
           }
 
-          // convert the MapWritable into a Map we can use
+          Cell value = regionInfoRow.get(COL_REGIONINFO);
-          SortedMap<Text, Cell> results = 
-            sortedMapFromMapWritable(regionInfoRow);
-
-          Cell value = results.get(COL_REGIONINFO);
 
           if (value == null || value.getValue().length == 0) {
             throw new IOException("HRegionInfo was null or empty in " + 
@@ -447,7 +422,7 @@ public class HConnectionManager implements HConstants {
           }
           
           String serverAddress = 
-            Writables.cellToString(results.get(COL_SERVER));
+            Writables.cellToString(regionInfoRow.get(COL_SERVER));
         
           if (serverAddress.equals("")) { 
             throw new NoServerForRegionException(

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

@@ -39,8 +39,6 @@ import org.apache.hadoop.hbase.filter.WhileMatchRowFilter;
 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.io.HbaseMapWritable;
-import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.util.Writables;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.io.Writable;
@@ -329,7 +327,7 @@ public class HTable implements HConstants {
    * @return Map of columns to values.  Map is empty if row does not exist.
    * @throws IOException
    */
-  public SortedMap<Text, Cell> getRow(final Text row) throws IOException {
+  public Map<Text, Cell> getRow(final Text row) throws IOException {
     return getRow(row, HConstants.LATEST_TIMESTAMP);
   }
 
@@ -341,24 +339,13 @@ public class HTable implements HConstants {
    * @return Map of columns to values.  Map is empty if row does not exist.
    * @throws IOException
    */
-  public SortedMap<Text, Cell> getRow(final Text row, final long ts) 
+  public Map<Text, Cell> getRow(final Text row, final long ts) 
   throws IOException {
-    HbaseMapWritable value = null;
+    return getRegionServerWithRetries(new ServerCallable<RowResult>(row) {
-         
+      public RowResult call() throws IOException {
-    value = getRegionServerWithRetries(new ServerCallable<HbaseMapWritable>(row) {
-      public HbaseMapWritable call() throws IOException {
         return server.getRow(location.getRegionInfo().getRegionName(), row, ts);
       }
     });
-    
-    SortedMap<Text, Cell> results = new TreeMap<Text, Cell>();
-    if (value != null && value.size() != 0) {
-      for (Map.Entry<Writable, Writable> e: value.entrySet()) {
-        HStoreKey key = (HStoreKey) e.getKey();
-        results.put(key.getColumn(), (Cell)e.getValue());
-      }
-    }
-    return results;
   }
 
   /** 
@@ -369,7 +356,7 @@ public class HTable implements HConstants {
    * @return Map of columns to values.  Map is empty if row does not exist.
    * @throws IOException
    */
-  public SortedMap<Text, Cell> getRow(final Text row, final Text[] columns) 
+  public Map<Text, Cell> getRow(final Text row, final Text[] columns) 
   throws IOException {
     return getRow(row, columns, HConstants.LATEST_TIMESTAMP);
   }
@@ -383,26 +370,15 @@ public class HTable implements HConstants {
    * @return Map of columns to values.  Map is empty if row does not exist.
    * @throws IOException
    */
-  public SortedMap<Text, Cell> getRow(final Text row, final Text[] columns, 
+  public Map<Text, Cell> getRow(final Text row, final Text[] columns, 
     final long ts) 
   throws IOException {       
-    HbaseMapWritable value = null;
+    return getRegionServerWithRetries(new ServerCallable<RowResult>(row) {
-         
+      public RowResult call() throws IOException {
-    value = getRegionServerWithRetries(new ServerCallable<HbaseMapWritable>(row) {
-      public HbaseMapWritable call() throws IOException {
         return server.getRow(location.getRegionInfo().getRegionName(), row, 
           columns, ts);
       }
     });
-    
-    SortedMap<Text, Cell> results = new TreeMap<Text, Cell>();
-    if (value != null && value.size() != 0) {
-      for (Map.Entry<Writable, Writable> e: value.entrySet()) {
-        HStoreKey key = (HStoreKey) e.getKey();
-        results.put(key.getColumn(), (Cell)e.getValue());
-      }
-    }
-    return results;
   }
 
   /** 

src/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java

@@ -26,7 +26,6 @@ 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.io.HbaseMapWritable;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.ipc.VersionedProtocol;
 import org.apache.hadoop.hbase.HRegionInfo;
@@ -100,7 +99,7 @@ public interface HRegionInterface extends VersionedProtocol {
    * @return map of values
    * @throws IOException
    */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, final long ts)
+  public RowResult getRow(final Text regionName, final Text row, final long ts)
   throws IOException;
 
   /**
@@ -112,7 +111,7 @@ public interface HRegionInterface extends VersionedProtocol {
    * @return map of values
    * @throws IOException
    */
-  public HbaseMapWritable getClosestRowBefore(final Text regionName, final Text row)
+  public RowResult getClosestRowBefore(final Text regionName, final Text row)
   throws IOException;
 
   /**
@@ -123,7 +122,7 @@ public interface HRegionInterface extends VersionedProtocol {
    * @return map of values
    * @throws IOException
    */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, 
+  public RowResult getRow(final Text regionName, final Text row, 
     final Text[] columns, final long ts)
   throws IOException;
 
@@ -135,7 +134,7 @@ public interface HRegionInterface extends VersionedProtocol {
    * @return map of values
    * @throws IOException
    */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, 
+  public RowResult getRow(final Text regionName, final Text row, 
     final Text[] columns)
   throws IOException;
 

src/java/org/apache/hadoop/hbase/regionserver/HRegion.java

@@ -28,6 +28,7 @@ import java.util.Set;
 import java.util.Random;
 import java.util.SortedMap;
 import java.util.TreeMap;
+import java.util.HashMap;
 import java.util.TreeSet;
 import java.util.Map.Entry;
 import java.util.concurrent.ConcurrentHashMap;
@@ -46,6 +47,8 @@ import org.apache.hadoop.hbase.filter.RowFilterInterface;
 import org.apache.hadoop.hbase.io.BatchOperation;
 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.io.HbaseMapWritable;
 import org.apache.hadoop.hbase.util.Writables;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.io.WritableUtils;
@@ -1158,7 +1161,7 @@ public class HRegion implements HConstants {
    * @return map of values
    * @throws IOException
    */
-  public Map<Text, Cell> getClosestRowBefore(final Text row)
+  public RowResult getClosestRowBefore(final Text row)
   throws IOException{
     // look across all the HStores for this region and determine what the
     // closest key is across all column families, since the data may be sparse
@@ -1192,13 +1195,16 @@ public class HRegion implements HConstants {
       }
       
       // now that we've found our key, get the values
-      TreeMap<Text, Cell> result = new TreeMap<Text, Cell>();
+      Map<Text, Cell> cells = new HashMap<Text, Cell>();
       for (Text colFamily: stores.keySet()) {
         HStore targetStore = stores.get(colFamily);
-        targetStore.getFull(key, null, result);
+        targetStore.getFull(key, null, cells);
       }
       
-      return result;
+      HbaseMapWritable cellsWritten = new HbaseMapWritable();
+      cellsWritten.putAll(cells);
+      
+      return new RowResult(key.getRow(), cellsWritten);
     } finally {
       lock.readLock().unlock();
     }

src/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -1000,20 +1000,20 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
   }
 
   /** {@inheritDoc} */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, final long ts)
+  public RowResult getRow(final Text regionName, final Text row, final long ts)
   throws IOException {
     return getRow(regionName, row, null, ts);
   }
 
   /** {@inheritDoc} */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, 
+  public RowResult getRow(final Text regionName, final Text row, 
     final Text[] columns)
   throws IOException {
     return getRow(regionName, row, columns, HConstants.LATEST_TIMESTAMP);
   }
 
   /** {@inheritDoc} */
-  public HbaseMapWritable getRow(final Text regionName, final Text row, 
+  public RowResult getRow(final Text regionName, final Text row, 
     final Text[] columns, final long ts)
   throws IOException {
     checkOpen();
@@ -1029,10 +1029,8 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
       HRegion region = getRegion(regionName);
       Map<Text, Cell> map = region.getFull(row, columnSet, ts);
       HbaseMapWritable result = new HbaseMapWritable();
-      for (Map.Entry<Text, Cell> es: map.entrySet()) {
+      result.putAll(map);
-        result.put(new HStoreKey(row, es.getKey()), es.getValue());
+      return new RowResult(row, result);
-      }
-      return result;
     } catch (IOException e) {
       checkFileSystem();
       throw e;
@@ -1040,7 +1038,7 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
   }
 
   /** {@inheritDoc} */
-  public HbaseMapWritable getClosestRowBefore(final Text regionName, 
+  public RowResult getClosestRowBefore(final Text regionName, 
     final Text row)
   throws IOException {
     checkOpen();
@@ -1048,18 +1046,9 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
     try {
       // locate the region we're operating on
       HRegion region = getRegion(regionName);
-      HbaseMapWritable result = new HbaseMapWritable();
-      // ask the region for all the data 
-      Map<Text, Cell> map = region.getClosestRowBefore(row);
-      // convert to a MapWritable
-      if (map == null) {
-        return null;
-      }
-      for (Map.Entry<Text, Cell> es: map.entrySet()) {
-        result.put(new HStoreKey(row, es.getKey()), es.getValue());
-      }
-      return result;
 
+      // ask the region for all the data 
+      return region.getClosestRowBefore(row);
     } catch (IOException e) {
       checkFileSystem();
       throw e;

src/java/org/apache/hadoop/hbase/regionserver/HStore.java

@@ -1092,7 +1092,7 @@ public class HStore implements HConstants {
    *
    * The returned object should map column names to Cells.
    */
-  void getFull(HStoreKey key, final Set<Text> columns, TreeMap<Text, Cell> results)
+  void getFull(HStoreKey key, final Set<Text> columns, Map<Text, Cell> results)
   throws IOException {
     Map<Text, Long> deletes = new HashMap<Text, Long>();
     
@@ -1124,7 +1124,7 @@ public class HStore implements HConstants {
   }
   
   private void getFullFromMapFile(MapFile.Reader map, HStoreKey key, 
-    Set<Text> columns, Map<Text, Long> deletes, TreeMap<Text, Cell> results) 
+    Set<Text> columns, Map<Text, Long> deletes, Map<Text, Cell> results) 
   throws IOException {
     synchronized(map) {
       // seek back to the beginning

src/java/org/apache/hadoop/hbase/regionserver/Memcache.java

@@ -145,7 +145,7 @@ class Memcache {
    * @param results
    */
   void getFull(HStoreKey key, Set<Text> columns, Map<Text, Long> deletes, 
-    SortedMap<Text, Cell> results) {
+    Map<Text, Cell> results) {
     this.lock.readLock().lock();
     try {
       synchronized (memcache) {
@@ -161,8 +161,7 @@ class Memcache {
   }
 
   private void internalGetFull(SortedMap<HStoreKey, byte[]> map, HStoreKey key, 
-    Set<Text> columns, Map<Text, Long> deletes, 
+    Set<Text> columns, Map<Text, Long> deletes, Map<Text, Cell> results) {
-    SortedMap<Text, Cell> results) {
 
     if (map.isEmpty() || key == null) {
       return;

src/java/org/apache/hadoop/hbase/thrift/ThriftServer.java

@@ -24,7 +24,7 @@ import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.SortedMap;
+import java.util.Map;
 import java.util.TreeMap;
 import java.util.Map.Entry;
 
@@ -275,9 +275,9 @@ public class ThriftServer {
       }
       try {
         HTable table = getTable(tableName);
-        SortedMap<Text, Cell> values = 
+        Map<Text, Cell> values = 
           table.getRow(getText(row), timestamp);
-        // copy the map from type <Text, byte[]> to <byte[], byte[]>
+        // copy the map from type <Text, Cell> to <byte[], byte[]>
         HashMap<byte[], byte[]> returnValues = new HashMap<byte[], byte[]>();
         for (Entry<Text, Cell> e : values.entrySet()) {
           returnValues.put(e.getKey().getBytes(), e.getValue().getValue());
@@ -472,7 +472,7 @@ public class ThriftServer {
       retval.row = key.getRow().getBytes();
       retval.columns = new HashMap<byte[], byte[]>(results.size());
       
-      for (SortedMap.Entry<Text, byte[]> e : results.entrySet()) {
+      for (Map.Entry<Text, byte[]> e : results.entrySet()) {
         retval.columns.put(e.getKey().getBytes(), e.getValue());
       }
       return retval;