You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2008/08/31 07:14:01 UTC

svn commit: r690640 - in /hadoop/hbase/branches/0.2: ./ src/java/org/apache/hadoop/hbase/ src/java/org/apache/hadoop/hbase/regionserver/ src/test/org/apache/hadoop/hbase/

Author: stack
Date: Sat Aug 30 22:14:00 2008
New Revision: 690640

URL: http://svn.apache.org/viewvc?rev=690640&view=rev
Log:
HBASE-832 Problem with row keys beginnig with characters < than ',' and the region location cache

Modified:
    hadoop/hbase/branches/0.2/CHANGES.txt
    hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HConstants.java
    hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HStoreKey.java
    hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java
    hadoop/hbase/branches/0.2/src/test/org/apache/hadoop/hbase/TestCompare.java

Modified: hadoop/hbase/branches/0.2/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/CHANGES.txt?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/CHANGES.txt (original)
+++ hadoop/hbase/branches/0.2/CHANGES.txt Sat Aug 30 22:14:00 2008
@@ -49,6 +49,8 @@
                (Jean-Daniel Cryans via Stack)
    HBASE-855   compaction can return less versions then we should in some cases
                (Billy Pearson via Stack)
+   HBASE-832   Problem with row keys beginnig with characters < than ',' and
+               the region location cache
 
   IMPROVEMENTS
    HBASE-801  When a table haven't disable, shell could response in a "user

Modified: hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HConstants.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HConstants.java?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HConstants.java (original)
+++ hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HConstants.java Sat Aug 30 22:14:00 2008
@@ -132,7 +132,10 @@
   static final byte [] ROOT_TABLE_NAME = Bytes.toBytes("-ROOT-");
 
   /** The META table's name. */
-  static final byte [] META_TABLE_NAME = Bytes.toBytes(".META.");
+  static final byte [] META_TABLE_NAME = Bytes.toBytes(".META.");  
+
+  /** delimiter used between portions of a region name */
+  public static final int META_ROW_DELIMITER = ',';
 
   // Defines for the column names used in both ROOT and META HBase 'meta' tables.
   

Modified: hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HStoreKey.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HStoreKey.java?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HStoreKey.java (original)
+++ hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/HStoreKey.java Sat Aug 30 22:14:00 2008
@@ -40,6 +40,12 @@
   private byte [] column = HConstants.EMPTY_BYTE_ARRAY;
   private long timestamp = Long.MAX_VALUE;
 
+  /*
+   * regionInfo is only used as a hack to compare HSKs.
+   * It is not serialized.  See https://issues.apache.org/jira/browse/HBASE-832
+   */
+  private HRegionInfo regionInfo = null;
+
   /** Default constructor used in conjunction with Writable interface */
   public HStoreKey() {
     super();
@@ -47,8 +53,8 @@
   
   /**
    * Create an HStoreKey specifying only the row
-   * The column defaults to the empty string and the time stamp defaults to
-   * Long.MAX_VALUE
+   * The column defaults to the empty string, the time stamp defaults to
+   * Long.MAX_VALUE and the table defaults to empty string
    * 
    * @param row - row key
    */
@@ -58,8 +64,8 @@
 
   /**
    * Create an HStoreKey specifying only the row
-   * The column defaults to the empty string and the time stamp defaults to
-   * Long.MAX_VALUE
+   * The column defaults to the empty string, the time stamp defaults to
+   * Long.MAX_VALUE and the table defaults to empty string
    * 
    * @param row - row key
    */
@@ -69,7 +75,7 @@
 
   /**
    * Create an HStoreKey specifying the row and timestamp
-   * The column name defaults to the empty string
+   * The column and table names default to the empty string
    * 
    * @param row row key
    * @param timestamp timestamp value
@@ -80,29 +86,31 @@
 
   /**
    * Create an HStoreKey specifying the row and timestamp
-   * The column name defaults to the empty string
+   * The column and table names default to the empty string
    * 
    * @param row row key
    * @param timestamp timestamp value
    */
   public HStoreKey(final String row, long timestamp) {
-    this (row, "", timestamp);
+    this (row, "", timestamp, new HRegionInfo());
   }
 
   /**
    * Create an HStoreKey specifying the row and column names
    * The timestamp defaults to LATEST_TIMESTAMP
+   * and table name defaults to the empty string
    * 
    * @param row row key
    * @param column column key
    */
   public HStoreKey(final String row, final String column) {
-    this(row, column, HConstants.LATEST_TIMESTAMP);
+    this(row, column, HConstants.LATEST_TIMESTAMP, new HRegionInfo());
   }
 
   /**
    * Create an HStoreKey specifying the row and column names
    * The timestamp defaults to LATEST_TIMESTAMP
+   * and table name defaults to the empty string
    * 
    * @param row row key
    * @param column column key
@@ -110,6 +118,19 @@
   public HStoreKey(final byte [] row, final byte [] column) {
     this(row, column, HConstants.LATEST_TIMESTAMP);
   }
+  
+  /**
+   * Create an HStoreKey specifying the row, column names and table name
+   * The timestamp defaults to LATEST_TIMESTAMP
+   * 
+   * @param row row key
+   * @param column column key
+   * @param regionInfo region info
+   */
+  public HStoreKey(final byte [] row, 
+      final byte [] column, final HRegionInfo regionInfo) {
+    this(row, column, HConstants.LATEST_TIMESTAMP, regionInfo);
+  }
 
   /**
    * Create an HStoreKey specifying all the fields
@@ -118,13 +139,16 @@
    * @param row row key
    * @param column column key
    * @param timestamp timestamp value
+   * @param regionInfo region info
    */
-  public HStoreKey(final String row, final String column, long timestamp) {
-    this (Bytes.toBytes(row), Bytes.toBytes(column), timestamp);
+  public HStoreKey(final String row, 
+      final String column, long timestamp, final HRegionInfo regionInfo) {
+    this (Bytes.toBytes(row), Bytes.toBytes(column), 
+        timestamp, regionInfo);
   }
 
   /**
-   * Create an HStoreKey specifying all the fields
+   * Create an HStoreKey specifying all the fields with unspecified table
    * Does not make copies of the passed byte arrays. Presumes the passed 
    * arrays immutable.
    * @param row row key
@@ -132,10 +156,25 @@
    * @param timestamp timestamp value
    */
   public HStoreKey(final byte [] row, final byte [] column, long timestamp) {
+    this(row, column, timestamp, null);
+  }
+  
+  /**
+   * Create an HStoreKey specifying all the fields with specified table
+   * Does not make copies of the passed byte arrays. Presumes the passed 
+   * arrays immutable.
+   * @param row row key
+   * @param column column key
+   * @param timestamp timestamp value
+   * @param regionInfo region info
+   */
+  public HStoreKey(final byte [] row, 
+      final byte [] column, long timestamp, final HRegionInfo regionInfo) {
     // Make copies
     this.row = row;
     this.column = column;
     this.timestamp = timestamp;
+    this.regionInfo = regionInfo;
   }
   
   /** @return Approximate size in bytes of this key. */
@@ -205,6 +244,11 @@
     return this.timestamp;
   }
   
+  /** @return value of regioninfo */
+  public HRegionInfo getHRegionInfo() {
+    return this.regionInfo;
+  }
+  
   /**
    * Compares the row and column of two keys
    * @param other Key to compare against. Compares row and column.
@@ -274,7 +318,7 @@
   /** {@inheritDoc} */
   public int compareTo(Object o) {
     HStoreKey other = (HStoreKey)o;
-    int result = Bytes.compareTo(this.row, other.row);
+    int result = compareTwoRowKeys(this.regionInfo, this.row, other.row);
     if (result != 0) {
       return result;
     }
@@ -419,6 +463,66 @@
     return Bytes.add(hsk.getRow(), hsk.getColumn());
   }
   
+  /**
+   * Utility method to compare two row keys.
+   * This is required because of the meta delimiters.
+   * This is a hack.
+   * @param regioninfo
+   * @param rowA
+   * @param rowB
+   * @return value of the comparison
+   */
+  public static int compareTwoRowKeys(HRegionInfo regionInfo, 
+      byte[] rowA, byte[] rowB) {
+    if(regionInfo != null && (regionInfo.isMetaRegion() ||
+        regionInfo.isRootRegion())) {
+      byte[][] keysA = stripStartKeyMeta(rowA);
+      byte[][] KeysB = stripStartKeyMeta(rowB);
+      int rowCompare = Bytes.compareTo(keysA[0], KeysB[0]);
+      if(rowCompare == 0)
+        rowCompare = Bytes.compareTo(keysA[1], KeysB[1]);
+      return rowCompare;
+    } else {
+      return Bytes.compareTo(rowA, rowB);
+    }
+  }
+  
+  /**
+   * Utility method to check if two row keys are equal.
+   * This is required because of the meta delimiters
+   * This is a hack
+   * @param regioninfo
+   * @param rowA
+   * @param rowB
+   * @return if it's equal
+   */
+  public static boolean equalsTwoRowKeys(HRegionInfo regionInfo, 
+      byte[] rowA, byte[] rowB) {
+    return rowA == null && rowB == null? true:
+      rowA == null && rowB != null? false:
+        rowA != null && rowB == null? false:
+          rowA.length != rowB.length? false:
+        compareTwoRowKeys(regionInfo,rowA,rowB) == 0;
+  }
+  
+  private static byte[][] stripStartKeyMeta(byte[] rowKey) {
+    int offset = -1;
+    for (int i = rowKey.length - 1; i > 0; i--) {
+      if (rowKey[i] == HConstants.META_ROW_DELIMITER) {
+        offset = i;
+        break;
+      }
+    }
+    byte [] row = new byte[offset];
+    System.arraycopy(rowKey, 0, row, 0,offset);
+    byte [] timestamp = new byte[rowKey.length - offset - 1];
+    System.arraycopy(rowKey, offset+1, timestamp, 0,rowKey.length - offset - 1);
+    byte[][] elements = new byte[2][];
+    elements[0] = row;
+    elements[1] = timestamp;
+    return elements;
+  }
+  
   // Writable
 
   /** {@inheritDoc} */
@@ -434,4 +538,4 @@
     this.column = Bytes.readByteArray(in);
     this.timestamp = in.readLong();
   }
-}
\ No newline at end of file
+}

Modified: hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sat Aug 30 22:14:00 2008
@@ -1258,13 +1258,14 @@
         // get the closest key
         byte [] closestKey = store.getRowKeyAtOrBefore(row);
         // if it happens to be an exact match, we can stop looping
-        if (Bytes.equals(row, closestKey)) {
+        if (HStoreKey.equalsTwoRowKeys(regionInfo,row, closestKey)) {
           key = new HStoreKey(closestKey);
           break;
         }
         // otherwise, we need to check if it's the max and move to the next
         if (closestKey != null 
-          && (key == null || Bytes.compareTo(closestKey, key.getRow()) > 0) ) {
+          && (key == null || HStoreKey.compareTwoRowKeys(
+              regionInfo,closestKey, key.getRow()) > 0) ) {
           key = new HStoreKey(closestKey);
         }
       }

Modified: hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java (original)
+++ hadoop/hbase/branches/0.2/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java Sat Aug 30 22:14:00 2008
@@ -37,6 +37,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HStoreKey;
 import org.apache.hadoop.hbase.io.Cell;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -53,6 +54,8 @@
   private final Log LOG = LogFactory.getLog(this.getClass().getName());
   
   private final long ttl;
+  
+  private HRegionInfo regionInfo;
 
   // Note that since these structures are always accessed with a lock held,
   // so no additional synchronization is required.
@@ -72,14 +75,17 @@
    */
   public Memcache() {
     this.ttl = HConstants.FOREVER;
+    this.regionInfo = null;
   }
 
   /**
    * Constructor.
    * @param ttl The TTL for cache entries, in milliseconds.
+   * @param regionInfo The HRI for this cache 
    */
-  public Memcache(final long ttl) {
+  public Memcache(final long ttl, HRegionInfo regionInfo) {
     this.ttl = ttl;
+    this.regionInfo = regionInfo;
   }
 
   /*
@@ -383,7 +389,8 @@
     // the search key, or a range of values between the first candidate key
     // and the ultimate search key (or the end of the cache)
     if (!tailMap.isEmpty() &&
-        Bytes.compareTo(tailMap.firstKey().getRow(), search_key.getRow()) <= 0) {
+        HStoreKey.compareTwoRowKeys(regionInfo, 
+            tailMap.firstKey().getRow(), search_key.getRow()) <= 0) {
       Iterator<HStoreKey> key_iterator = tailMap.keySet().iterator();
 
       // Keep looking at cells as long as they are no greater than the 
@@ -391,9 +398,11 @@
       HStoreKey deletedOrExpiredRow = null;
       for (HStoreKey found_key = null; key_iterator.hasNext() &&
           (found_key == null ||
-            Bytes.compareTo(found_key.getRow(), row) <= 0);) {
+            HStoreKey.compareTwoRowKeys(regionInfo, 
+                found_key.getRow(), row) <= 0);) {
         found_key = key_iterator.next();
-        if (Bytes.compareTo(found_key.getRow(), row) <= 0) {
+        if (HStoreKey.compareTwoRowKeys(regionInfo, 
+            found_key.getRow(), row) <= 0) {
           if (HLogEdit.isDeleted(tailMap.get(found_key))) {
             HStore.handleDeleted(found_key, candidateKeys, deletes);
             if (deletedOrExpiredRow == null) {

Modified: hadoop/hbase/branches/0.2/src/test/org/apache/hadoop/hbase/TestCompare.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.2/src/test/org/apache/hadoop/hbase/TestCompare.java?rev=690640&r1=690639&r2=690640&view=diff
==============================================================================
--- hadoop/hbase/branches/0.2/src/test/org/apache/hadoop/hbase/TestCompare.java (original)
+++ hadoop/hbase/branches/0.2/src/test/org/apache/hadoop/hbase/TestCompare.java Sat Aug 30 22:14:00 2008
@@ -54,6 +54,43 @@
   }
   
   /**
+   * Tests cases where rows keys have characters below the ','.
+   * See HBASE-832
+   */
+  public void testHStoreKeyBorderCases() {
+    HRegionInfo info = new HRegionInfo(new HTableDescriptor("testtable"),
+        HConstants.EMPTY_BYTE_ARRAY,HConstants.EMPTY_BYTE_ARRAY);
+    HStoreKey rowA = new HStoreKey("testtable,www.hbase.org/,1234",
+        "", Long.MAX_VALUE, info);
+    HStoreKey rowB = new HStoreKey("testtable,www.hbase.org/%20,99999",
+        "", Long.MAX_VALUE, info);
+
+    assertTrue(rowA.compareTo(rowB) > 0);
+
+    rowA = new HStoreKey("testtable,www.hbase.org/,1234",
+        "", Long.MAX_VALUE, HRegionInfo.FIRST_META_REGIONINFO);
+    rowB = new HStoreKey("testtable,www.hbase.org/%20,99999",
+        "", Long.MAX_VALUE, HRegionInfo.FIRST_META_REGIONINFO);
+
+    assertTrue(rowA.compareTo(rowB) < 0);
+
+    rowA = new HStoreKey("testtable,,1234",
+        "", Long.MAX_VALUE, HRegionInfo.FIRST_META_REGIONINFO);
+    rowB = new HStoreKey("testtable,$www.hbase.org/,99999",
+        "", Long.MAX_VALUE, HRegionInfo.FIRST_META_REGIONINFO);
+
+    assertTrue(rowA.compareTo(rowB) < 0);
+
+    rowA = new HStoreKey(".META.,testtable,www.hbase.org/,1234,4321",
+        "", Long.MAX_VALUE, HRegionInfo.ROOT_REGIONINFO);
+    rowB = new HStoreKey(".META.,testtable,www.hbase.org/%20,99999,99999",
+        "", Long.MAX_VALUE, HRegionInfo.ROOT_REGIONINFO);
+
+    assertTrue(rowA.compareTo(rowB) > 0);
+  }
+
+  
+  /**
    * Sort of HRegionInfo.
    */
   public void testHRegionInfo() {