HBASE-1949 KeyValue expiration by Time-to-Live during major compaction is broken

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

CHANGES.txt

@@ -94,6 +94,8 @@ Release 0.21.0 - Unreleased
                when deleting a lot of values
    HBASE-1781  Weird behavior of WildcardColumnTracker.checkColumn(), 
                looks like recursive loop
+   HBASE-1949  KeyValue expiration by Time-to-Live during major compaction is
+               broken (Gary Helmling via Stack)
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

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

@@ -231,8 +231,10 @@ public class QueryMatcher {
      */
     long timestamp = Bytes.toLong(bytes, offset);
     if(isExpired(timestamp)) {
-      // reached the expired part, for scans, this indicates we're done.
+      /* KeyValue is expired, skip but don't early out since a non-expired
-      return MatchCode.NEXT;  // done_row
+       * kv could come next.
+       */
+      return MatchCode.SKIP;  // go to next kv
     }
     offset += Bytes.SIZEOF_LONG;
 

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

@@ -122,9 +122,8 @@ public class ScanQueryMatcher extends QueryMatcher {
     
     long timestamp = kv.getTimestamp();
     if (isExpired(timestamp)) {
-      // done, the rest wil also be expired as well.
+      // done, the rest of this column will also be expired as well.
-      stickyNextRow = true;
+      return MatchCode.SEEK_NEXT_COL;
-      return MatchCode.SEEK_NEXT_ROW;
     }
 
     byte type = kv.getType();

src/test/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java

@@ -27,6 +27,7 @@ import java.util.List;
 import org.apache.hadoop.hbase.HBaseTestCase;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.KeyValueTestUtil;
 import org.apache.hadoop.hbase.KeyValue.KeyComparator;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.regionserver.QueryMatcher.MatchCode;
@@ -162,4 +163,103 @@ implements HConstants {
     }
   }
 
+  
+  /**
+   * Verify that {@link QueryMatcher} only skips expired KeyValue 
+   * instances and does not exit early from the row (skipping 
+   * later non-expired KeyValues).  This version mimics a Get with
+   * explicitly specified column qualifiers.
+   * 
+   * @throws IOException
+   */
+  public void testMatch_ExpiredExplicit()
+  throws IOException {
+    
+    long testTTL = 1000;
+    MatchCode [] expected = new MatchCode[] {
+        MatchCode.SKIP,
+        MatchCode.INCLUDE,
+        MatchCode.SKIP,
+        MatchCode.INCLUDE,
+        MatchCode.SKIP,
+        MatchCode.NEXT
+    };
+        
+    QueryMatcher qm = new QueryMatcher(get, fam2,
+        get.getFamilyMap().get(fam2), testTTL, rowComparator, 1);
+    
+    long now = System.currentTimeMillis();
+    KeyValue [] kvs = new KeyValue[] {
+        new KeyValue(row1, fam2, col1, now-100, data),
+        new KeyValue(row1, fam2, col2, now-50, data),
+        new KeyValue(row1, fam2, col3, now-5000, data),
+        new KeyValue(row1, fam2, col4, now-500, data),
+        new KeyValue(row1, fam2, col5, now-10000, data),
+        new KeyValue(row2, fam1, col1, now-10, data)        
+    };
+
+    List<MatchCode> actual = new ArrayList<MatchCode>(kvs.length);
+    for (KeyValue kv : kvs) {
+      actual.add( qm.match(kv) );
+    }
+    
+    assertEquals(expected.length, actual.size());
+    for (int i=0; i<expected.length; i++) {
+      if(PRINT){
+        System.out.println("expected "+expected[i]+ 
+            ", actual " +actual.get(i));
+      }
+      assertEquals(expected[i], actual.get(i));
+    }
+  }
+  
+  
+  /**
+   * Verify that {@link QueryMatcher} only skips expired KeyValue 
+   * instances and does not exit early from the row (skipping 
+   * later non-expired KeyValues).  This version mimics a Get with
+   * wildcard-inferred column qualifiers.
+   * 
+   * @throws IOException
+   */ 
+  public void testMatch_ExpiredWildcard()
+  throws IOException {
+    
+    long testTTL = 1000;
+    MatchCode [] expected = new MatchCode[] {
+        MatchCode.INCLUDE,
+        MatchCode.INCLUDE,
+        MatchCode.SKIP,
+        MatchCode.INCLUDE,
+        MatchCode.SKIP,
+        MatchCode.NEXT
+    };
+        
+    QueryMatcher qm = new QueryMatcher(get, fam2,
+        null, testTTL, rowComparator, 1);
+    
+    long now = System.currentTimeMillis();
+    KeyValue [] kvs = new KeyValue[] {
+        new KeyValue(row1, fam2, col1, now-100, data),
+        new KeyValue(row1, fam2, col2, now-50, data),
+        new KeyValue(row1, fam2, col3, now-5000, data),
+        new KeyValue(row1, fam2, col4, now-500, data),
+        new KeyValue(row1, fam2, col5, now-10000, data),
+        new KeyValue(row2, fam1, col1, now-10, data)        
+    };
+
+    List<MatchCode> actual = new ArrayList<MatchCode>(kvs.length);
+    for (KeyValue kv : kvs) {
+      actual.add( qm.match(kv) );
+    }
+    
+    assertEquals(expected.length, actual.size());
+    for (int i=0; i<expected.length; i++) {
+      if(PRINT){
+        System.out.println("expected "+expected[i]+ 
+            ", actual " +actual.get(i));
+      }
+      assertEquals(expected[i], actual.get(i));
+    }
+  }
 }

src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java

@@ -354,4 +354,46 @@ public class TestStoreScanner extends TestCase {
     results.clear();
     assertEquals(false, scan.next(results));
   }
+  
+  /**
+   * Test expiration of KeyValues in combination with a configured TTL for 
+   * a column family (as should be triggered in a major compaction).
+   */
+  public void testWildCardTtlScan() throws IOException {
+    long now = System.currentTimeMillis();
+    KeyValue [] kvs = new KeyValue[] {
+        KeyValueTestUtil.create("R1", "cf", "a", now-1000, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R1", "cf", "b", now-10, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R1", "cf", "c", now-200, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R1", "cf", "d", now-10000, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R2", "cf", "a", now, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R2", "cf", "b", now-10, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R2", "cf", "c", now-200, KeyValue.Type.Put, "dont-care"),
+        KeyValueTestUtil.create("R2", "cf", "c", now-1000, KeyValue.Type.Put, "dont-care")
+    };
+    KeyValueScanner [] scanners = new KeyValueScanner[] {
+        new KeyValueScanFixture(KeyValue.COMPARATOR, kvs)
+    };
+    Scan scan = new Scan();
+    scan.setMaxVersions(1);
+    StoreScanner scanner =
+      new StoreScanner(scan, CF, 500, KeyValue.COMPARATOR,
+          null, scanners);
+
+    List<KeyValue> results = new ArrayList<KeyValue>();
+    assertEquals(true, scanner.next(results));
+    assertEquals(2, results.size());
+    assertEquals(kvs[1], results.get(0));
+    assertEquals(kvs[2], results.get(1));
+    results.clear();
+    
+    assertEquals(true, scanner.next(results));
+    assertEquals(3, results.size());
+    assertEquals(kvs[4], results.get(0));
+    assertEquals(kvs[5], results.get(1));
+    assertEquals(kvs[6], results.get(2));
+    results.clear();
+    
+    assertEquals(false, scanner.next(results));
+  }
 }