hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bryanduxb...@apache.org
Subject svn commit: r638525 - in /hadoop/hbase/trunk: ./ src/java/org/apache/hadoop/hbase/regionserver/ src/test/org/apache/hadoop/hbase/regionserver/
Date Tue, 18 Mar 2008 19:34:35 GMT
Author: bryanduxbury
Date: Tue Mar 18 12:34:33 2008
New Revision: 638525

URL: http://svn.apache.org/viewvc?rev=638525&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/trunk/CHANGES.txt
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HStore.java
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java
    hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestGet2.java
    hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestHMemcache.java

Modified: hadoop/hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/CHANGES.txt?rev=638525&r1=638524&r2=638525&view=diff
==============================================================================
--- hadoop/hbase/trunk/CHANGES.txt (original)
+++ hadoop/hbase/trunk/CHANGES.txt Tue Mar 18 12:34:33 2008
@@ -45,6 +45,7 @@
    HBASE-516   HStoreFile.finalKey does not update the final key if it is not
                the top region of a split region
    HBASE-525   HTable.getRow(Text) does not work (Clint Morgan via Bryan Duxbury)
+   HBASE-524   Problems with getFull
    
   IMPROVEMENTS
    HBASE-415   Rewrite leases to use DelayedBlockingQueue instead of polling

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HStore.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HStore.java?rev=638525&r1=638524&r2=638525&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HStore.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HStore.java Tue Mar 18
12:34:33 2008
@@ -1057,7 +1057,7 @@
    */
   void getFull(HStoreKey key, final Set<Text> columns, TreeMap<Text, Cell> results)
   throws IOException {
-    Map<Text, List<Long>> deletes = new HashMap<Text, List<Long>>();
+    Map<Text, Long> deletes = new HashMap<Text, Long>();
     
     // if the key is null, we're not even looking for anything. return.
     if (key == null) {
@@ -1067,7 +1067,7 @@
     this.lock.readLock().lock();
     
     // get from the memcache first.
-    memcache.getFull(key, columns, results);
+    memcache.getFull(key, columns, deletes, results);
     
     try {
       MapFile.Reader[] maparray = getReaders();
@@ -1078,37 +1078,66 @@
         
         // synchronize on the map so that no one else iterates it at the same 
         // time
-        synchronized(map) {
-          // seek back to the beginning
-          map.reset();
-          
-          // seek to the closest key that should match the row we're looking for
-          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;
-              }
-              if (columns == null || columns.contains(readkey.getColumn())) {
-                results.put(new Text(readcol), new Cell(readval.get(), readkey.getTimestamp()));
-              }
-              readval = new ImmutableBytesWritable();
-            } else if(key.getRow().compareTo(readkey.getRow()) < 0) {
-              break;
-            }
-            
-          } while(map.next(readkey, readval));
-        }
+        getFullFromMapFile(map, key, columns, deletes, results);
       }
       
     } finally {
       this.lock.readLock().unlock();
+    }
+  }
+  
+  private void getFullFromMapFile(MapFile.Reader map, HStoreKey key, 
+    Set<Text> columns, Map<Text, Long> deletes, TreeMap<Text, Cell> results)

+  throws IOException {
+    synchronized(map) {
+      // seek back to the beginning
+      map.reset();
+      
+      // seek to the closest key that should match the row we're looking for
+      ImmutableBytesWritable readval = new ImmutableBytesWritable();
+      HStoreKey readkey = (HStoreKey)map.getClosest(key, readval);
+      if (readkey == null) {
+        return;
+      }
+      do {
+        Text readcol = readkey.getColumn();
+        
+        // if we're looking for this column (or all of them), and there isn't 
+        // already a value for this column in the results map, and the key we 
+        // just read matches, then we'll consider it
+        if ((columns == null || columns.contains(readcol)) 
+          && !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), 
+                new Cell(readval.get(), readkey.getTimestamp()));
+              // 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
+          break;
+        }
+        
+      } while(map.next(readkey, readval));
     }
   }
   

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java?rev=638525&r1=638524&r2=638525&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java Tue Mar
18 12:34:33 2008
@@ -145,14 +145,15 @@
    * @param key
    * @param results
    */
-  void getFull(HStoreKey key, Set<Text> columns, SortedMap<Text, Cell> results)
{
+  void getFull(HStoreKey key, Set<Text> columns, Map<Text, Long> deletes, 
+    SortedMap<Text, Cell> results) {
     this.lock.readLock().lock();
     try {
       synchronized (memcache) {
-        internalGetFull(memcache, key, columns, results);
+        internalGetFull(memcache, key, columns, deletes, results);
       }
       synchronized (snapshot) {
-        internalGetFull(snapshot, key, columns, results);
+        internalGetFull(snapshot, key, columns, deletes, results);
       }
 
     } finally {
@@ -161,7 +162,8 @@
   }
 
   private void internalGetFull(SortedMap<HStoreKey, byte[]> map, HStoreKey key, 
-    Set<Text> columns, SortedMap<Text, Cell> results) {
+    Set<Text> columns, Map<Text, Long> deletes, 
+    SortedMap<Text, Cell> results) {
 
     if (map.isEmpty() || key == null) {
       return;
@@ -174,12 +176,17 @@
       if (results.get(itCol) == null && key.matchesWithoutColumn(itKey)) {
         byte [] val = tailMap.get(itKey);
 
-        if (!HLogEdit.isDeleted(val)) {
-          if (columns == null || columns.contains(itKey.getColumn())) {
-            results.put(itCol, new Cell(val, itKey.getTimestamp()));
+        if (columns == null || columns.contains(itKey.getColumn())) {
+          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), new Cell(val, itKey.getTimestamp()));
           }
         }
-
       } else if (key.getRow().compareTo(itKey.getRow()) < 0) {
         break;
       }

Modified: hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestGet2.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestGet2.java?rev=638525&r1=638524&r2=638525&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestGet2.java (original)
+++ hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestGet2.java Tue Mar
18 12:34:33 2008
@@ -23,6 +23,7 @@
 import java.util.Map;
 import java.util.HashSet;
 import java.util.TreeMap;
+import java.util.Set;
 
 import org.apache.hadoop.dfs.MiniDFSCluster;
 import org.apache.hadoop.hbase.filter.StopRowFilter;
@@ -40,7 +41,7 @@
  * {@link TestGet} is a medley of tests of get all done up as a single test.
  * This class 
  */
-public class TestGet2 extends HBaseTestCase {
+public class TestGet2 extends HBaseTestCase implements HConstants {
   private MiniDFSCluster miniHdfs;
 
   @Override
@@ -263,7 +264,7 @@
       }
     }    
   }
-    
+  
   private void assertSpecifiedColumns(final HRegion region, final Text row) 
   throws IOException {
     HashSet<Text> all = new HashSet<Text>();
@@ -300,6 +301,111 @@
     assertNull(result.get(COLUMNS[1]));
     assertNull(result.get(COLUMNS[2]));    
   }  
+  
+  public void testGetFullMultiMapfile() throws IOException {
+    HRegion region = null;
+    HRegionIncommon region_incommon = null;
+    BatchUpdate batchUpdate = null;
+    Map<Text, Cell> results = null;
+    
+    try {
+      HTableDescriptor htd = createTableDescriptor(getName());
+      region = createNewHRegion(htd, null, null);
+      region_incommon = new HRegionIncommon(region);
+           
+      //
+      // Test ordering issue
+      //
+      Text row = new Text("row1");
+     
+      // write some data
+      batchUpdate = new BatchUpdate(row);
+      batchUpdate.put(COLUMNS[0], "olderValue".getBytes());
+      region.batchUpdate(batchUpdate);
+
+      // flush
+      region.flushcache();
+      
+      // assert that getFull gives us the older value
+      results = region.getFull(row, (Set<Text>)null, LATEST_TIMESTAMP);
+      assertEquals("olderValue", new String(results.get(COLUMNS[0]).getValue()));
+      
+      // write a new value for the cell
+      batchUpdate = new BatchUpdate(row);
+      batchUpdate.put(COLUMNS[0], "newerValue".getBytes());
+      region.batchUpdate(batchUpdate);
+      
+      // flush
+      region.flushcache();
+      
+      // assert that getFull gives us the later value
+      results = region.getFull(row, (Set<Text>)null, LATEST_TIMESTAMP);
+      assertEquals("newerValue", new String(results.get(COLUMNS[0]).getValue()));
+     
+      //
+      // 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
+      batchUpdate = new BatchUpdate(row2);
+      batchUpdate.put(cell1, "column0 value".getBytes());
+      batchUpdate.put(cell2, "column1 value".getBytes());
+      region.batchUpdate(batchUpdate);
+      
+      // flush
+      region.flushcache();
+      
+      // assert i get both columns
+      results = region.getFull(row2, (Set<Text>)null, LATEST_TIMESTAMP);
+      assertEquals("Should have two columns in the results map", 2, results.size());
+      assertEquals("column0 value", new String(results.get(cell1).getValue()));
+      assertEquals("column1 value", new String(results.get(cell2).getValue()));
+      
+      // write a delete for the first column
+      batchUpdate = new BatchUpdate(row2);
+      batchUpdate.delete(cell1);
+      batchUpdate.put(cell2, "column1 new value".getBytes());      
+      region.batchUpdate(batchUpdate);
+            
+      // flush
+      region.flushcache(); 
+      
+      // assert i get the second column only
+      results = region.getFull(row2, (Set<Text>)null, LATEST_TIMESTAMP);
+      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).getValue()));
+      
+      //
+      // Include a delete and value from the memcache in the mix
+      //
+      batchUpdate = new BatchUpdate(row2);
+      batchUpdate.delete(cell2);      
+      batchUpdate.put(cell3, "column2 value!".getBytes());
+      region.batchUpdate(batchUpdate);
+      
+      // assert i get the third column only
+      results = region.getFull(row2, (Set<Text>)null, LATEST_TIMESTAMP);
+      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).getValue()));
+      
+    } finally {
+      if (region != null) {
+        try {
+          region.close();
+        } catch (Exception e) {
+          e.printStackTrace();
+        }
+        region.getLog().closeAndDelete();
+      }
+    }  
+  }
   
   private void assertColumnsPresent(final HRegion r, final Text row)
   throws IOException {

Modified: hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestHMemcache.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestHMemcache.java?rev=638525&r1=638524&r2=638525&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestHMemcache.java (original)
+++ hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/regionserver/TestHMemcache.java Tue
Mar 18 12:34:33 2008
@@ -136,7 +136,8 @@
     for (int i = 0; i < ROW_COUNT; i++) {
       HStoreKey hsk = new HStoreKey(getRowName(i));
       TreeMap<Text, Cell> all = new TreeMap<Text, Cell>();
-      this.hmemcache.getFull(hsk, null, all);
+      TreeMap<Text, Long> deletes = new TreeMap<Text, Long>();
+      this.hmemcache.getFull(hsk, null, deletes, all);
       isExpectedRow(i, all);
     }
   }



Mime
View raw message