hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bryanduxb...@apache.org
Subject svn commit: r638524 - in /hadoop/hbase/branches/0.1: CHANGES.txt src/java/org/apache/hadoop/hbase/HStore.java src/test/org/apache/hadoop/hbase/TestGet2.java src/test/org/apache/hadoop/hbase/TestHMemcache.java
Date Tue, 18 Mar 2008 19:33:10 GMT
Author: bryanduxbury
Date: Tue Mar 18 12:33:08 2008
New Revision: 638524

URL: http://svn.apache.org/viewvc?rev=638524&view=rev
Log:
HBASE-524 Problems with getFull
-Added new test case to exercise the problems
-Fixed getFull implementation in HStore and Memcache

Modified:
    hadoop/hbase/branches/0.1/CHANGES.txt
    hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
    hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestGet2.java
    hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHMemcache.java

Modified: hadoop/hbase/branches/0.1/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/CHANGES.txt?rev=638524&r1=638523&r2=638524&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/CHANGES.txt (original)
+++ hadoop/hbase/branches/0.1/CHANGES.txt Tue Mar 18 12:33:08 2008
@@ -45,7 +45,8 @@
                startcode of -1 in .META.
    HBASE-516   HStoreFile.finalKey does not update the final key if it is not
                the top region of a split region
-
+   HBASE-524   Problems with getFull
+   
   IMPROVEMENTS
    HADOOP-2555 Refactor the HTable#get and HTable#getRow methods to avoid
                repetition of retry-on-failure logic (thanks to Peter Dolan and

Modified: hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java?rev=638524&r1=638523&r2=638524&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java (original)
+++ hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java Tue Mar 18 12:33:08
2008
@@ -173,14 +173,16 @@
      * @param key
      * @param results
      */
-    void getFull(HStoreKey key, SortedMap<Text, byte[]> results) {
+    void getFull(HStoreKey key, Map<Text, Long> deletes, 
+      SortedMap<Text, byte[]> results) {
+      
       this.lock.readLock().lock();
       try {
         synchronized (memcache) {
-          internalGetFull(memcache, key, results);
+          internalGetFull(memcache, key, deletes, results);
         }
         synchronized (snapshot) {
-          internalGetFull(snapshot, key, results);
+          internalGetFull(snapshot, key, deletes, results);
         }
 
       } finally {
@@ -189,7 +191,7 @@
     }
 
     private void internalGetFull(SortedMap<HStoreKey, byte []> map, HStoreKey key,

-        SortedMap<Text, byte []> results) {
+      Map<Text, Long> deletes, SortedMap<Text, byte []> results) {
 
       if (map.isEmpty() || key == null) {
         return;
@@ -202,10 +204,15 @@
         if (results.get(itCol) == null && key.matchesWithoutColumn(itKey)) {
           byte [] val = tailMap.get(itKey);
 
-          if (!HLogEdit.isDeleted(val)) {
-            results.put(itCol, val);
+          if (HLogEdit.isDeleted(val)) {
+            if (!deletes.containsKey(itCol) 
+              || deletes.get(itCol).longValue() < itKey.getTimestamp()) {
+              deletes.put(new Text(itCol), itKey.getTimestamp());
+            }
+          } else if (!(deletes.containsKey(itCol) 
+            && deletes.get(itCol).longValue() >= itKey.getTimestamp())) {
+            results.put(new Text(itCol), val);
           }
-
         } else if (key.getRow().compareTo(itKey.getRow()) < 0) {
           break;
         }
@@ -1576,46 +1583,73 @@
    * The returned object should map column names to byte arrays (byte[]).
    */
   void getFull(HStoreKey key, TreeMap<Text, byte []> results)
-    throws IOException {
-    Map<Text, List<Long>> deletes = new HashMap<Text, List<Long>>();
+  throws IOException {
+    Map<Text, Long> deletes = new HashMap<Text, Long>();
     
     if (key == null) {
       return;
     }
     
     this.lock.readLock().lock();
-    memcache.getFull(key, results);
+    memcache.getFull(key, deletes, results);
     try {
       MapFile.Reader[] maparray = getReaders();
       for (int i = maparray.length - 1; i >= 0; i--) {
         MapFile.Reader map = maparray[i];
-        synchronized(map) {
-          map.reset();
-          ImmutableBytesWritable readval = new ImmutableBytesWritable();
-          HStoreKey readkey = (HStoreKey)map.getClosest(key, readval);
-          if (readkey == null) {
-            continue;
-          }
-          do {
-            Text readcol = readkey.getColumn();
-            if (results.get(readcol) == null
-                && key.matchesWithoutColumn(readkey)) {
-              if(isDeleted(readkey, readval.get(), true, deletes)) {
-                break;
-              }
-              results.put(new Text(readcol), readval.get());
-              readval = new ImmutableBytesWritable();
-            } else if(key.getRow().compareTo(readkey.getRow()) < 0) {
-              break;
-            }
-            
-          } while(map.next(readkey, readval));
-        }
+        getFullFromMapFile(map, key, deletes, results);
       }
-      
     } finally {
       this.lock.readLock().unlock();
     }
+  }
+  
+  private void getFullFromMapFile(MapFile.Reader map, HStoreKey key, 
+    Map<Text, Long> deletes, TreeMap<Text, byte[]> results) 
+  throws IOException {
+    
+    synchronized(map) {
+      map.reset();
+      ImmutableBytesWritable readval = new ImmutableBytesWritable();
+      HStoreKey readkey = (HStoreKey)map.getClosest(key, readval);
+      if (readkey == null) {
+        return;
+      }
+      do {
+        Text readcol = readkey.getColumn();
+        
+        // if there isn't already a value in the results map, and the key we 
+        // just read matches, then we'll consider it
+        if (!results.containsKey(readcol) && key.matchesWithoutColumn(readkey)) {
+          // if the value of the cell we're looking at right now is a delete, 
+          // we need to treat it differently
+          if(HLogEdit.isDeleted(readval.get())) {
+            // if it's not already recorded as a delete or recorded with a more
+            // recent delete timestamp, record it for later
+            if (!deletes.containsKey(readcol) 
+              || deletes.get(readcol).longValue() < readkey.getTimestamp()) {
+              deletes.put(new Text(readcol), readkey.getTimestamp());              
+            }
+          } else if (!(deletes.containsKey(readcol) 
+            && deletes.get(readcol).longValue() >= readkey.getTimestamp()) ) {
+            // So the cell itself isn't a delete, but there may be a delete 
+            // pending from earlier in our search. Only record this result if
+            // there aren't any pending deletes.
+            if (!(deletes.containsKey(readcol) 
+              && deletes.get(readcol).longValue() >= readkey.getTimestamp()))
{
+              results.put(new Text(readcol), readval.get());
+              // need to reinstantiate the readval so we can reuse it, 
+              // otherwise next iteration will destroy our result
+              readval = new ImmutableBytesWritable();
+            }
+          } 
+        } else if(key.getRow().compareTo(readkey.getRow()) < 0) {
+          // if we've crossed into the next row, then we can just stop 
+          // iterating
+          return;
+        }
+        
+      } while(map.next(readkey, readval));
+    }    
   }
   
   MapFile.Reader [] getReaders() {

Modified: hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestGet2.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestGet2.java?rev=638524&r1=638523&r2=638524&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestGet2.java (original)
+++ hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestGet2.java Tue Mar 18 12:33:08
2008
@@ -218,6 +218,111 @@
     }
   }
   
+  public void testGetFullMultiMapfile() throws IOException {
+    HRegion region = null;
+    HRegionIncommon region_incommon = null;
+    Map<Text, byte[]> results = null;
+    
+    try {
+      HTableDescriptor htd = createTableDescriptor(getName());
+      HRegionInfo hri = new HRegionInfo(htd, null, null);
+      region = createNewHRegion(htd, null, null);
+      region_incommon = new HRegionIncommon(region);
+     
+      //
+      // Test ordering issue
+      //
+      Text row = new Text("row1");
+     
+      // write some data
+      long lockid = region_incommon.startBatchUpdate(row);
+      region_incommon.put(lockid, COLUMNS[0], "olderValue".getBytes());
+      region_incommon.commit(lockid);
+
+      // flush
+      region.flushcache();
+      
+      // assert that getFull gives us the older value
+      results = region.getFull(row);
+      assertEquals("olderValue", new String(results.get(COLUMNS[0])));
+      
+      // write a new value for the cell
+      lockid = region_incommon.startBatchUpdate(row);
+      region_incommon.put(lockid, COLUMNS[0], "newerValue".getBytes());
+      region_incommon.commit(lockid);
+      
+      // flush
+      region.flushcache();
+      
+      // assert that getFull gives us the later value
+      results = region.getFull(row);
+      assertEquals("newerValue", new String(results.get(COLUMNS[0])));
+     
+      //
+      // Test the delete masking issue
+      //
+      Text row2 = new Text("row2");
+      Text cell1 = new Text(COLUMNS[0].toString() + "a");
+      Text cell2 = new Text(COLUMNS[0].toString() + "b");
+      Text cell3 = new Text(COLUMNS[0].toString() + "c");
+      
+      // write some data at two columns
+      lockid = region_incommon.startBatchUpdate(row2);
+      region_incommon.put(lockid, cell1, "column0 value".getBytes());
+      region_incommon.put(lockid, cell2, "column1 value".getBytes());
+      region_incommon.commit(lockid);
+      
+      // flush
+      region.flushcache();
+      
+      // assert i get both columns
+      results = region.getFull(row2);
+      assertEquals("Should have two columns in the results map", 2, results.size());
+      assertEquals("column0 value", new String(results.get(cell1)));
+      assertEquals("column1 value", new String(results.get(cell2)));
+      
+      // write a delete for the first column
+      lockid = region_incommon.startBatchUpdate(row2);
+      region_incommon.delete(lockid, cell1);
+      region_incommon.put(lockid, cell2, "column1 new value".getBytes());      
+      region_incommon.commit(lockid);
+            
+      // flush
+      region.flushcache(); 
+      
+      // assert i get the second column only
+      results = region.getFull(row2);
+      assertEquals("Should have one column in the results map", 1, results.size());
+      assertNull("column0 value", results.get(cell1));
+      assertEquals("column1 new value", new String(results.get(cell2)));
+      
+      //
+      // Include a delete and value from the memcache in the mix
+      //
+      lockid = region_incommon.startBatchUpdate(row2);
+      region_incommon.delete(lockid, cell2);      
+      region_incommon.put(lockid, cell3, "column2 value!".getBytes());
+      region_incommon.commit(lockid);
+      
+      // assert i get the third column only
+      results = region.getFull(row2);
+      assertEquals("Should have one column in the results map", 1, results.size());
+      assertNull("column0 value", results.get(cell1));
+      assertNull("column1 value", results.get(cell2));
+      assertEquals("column2 value!", new String(results.get(cell3)));
+      
+    } finally {
+      if (region != null) {
+        try {
+          region.close();
+        } catch (Exception e) {
+          e.printStackTrace();
+        }
+        region.getLog().closeAndDelete();
+      }
+    }  
+  }
+  
   
   private void assertCellValueEquals(final HRegion region, final Text row,
     final Text column, final long timestamp, final String value)

Modified: hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHMemcache.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHMemcache.java?rev=638524&r1=638523&r2=638524&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHMemcache.java (original)
+++ hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHMemcache.java Tue Mar
18 12:33:08 2008
@@ -123,7 +123,8 @@
     for (int i = 0; i < ROW_COUNT; i++) {
       HStoreKey hsk = new HStoreKey(getRowName(i));
       TreeMap<Text, byte []> all = new TreeMap<Text, byte[]>();
-      this.hmemcache.getFull(hsk, all);
+      Map<Text, Long> deletes = new TreeMap<Text, Long>();
+      this.hmemcache.getFull(hsk, deletes, all);
       isExpectedRow(i, all);
     }
   }



Mime
View raw message