You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ns...@apache.org on 2011/10/11 21:12:57 UTC

svn commit: r1182031 - in /hbase/branches/0.89/src: main/java/org/apache/hadoop/hbase/ main/java/org/apache/hadoop/hbase/regionserver/ test/java/org/apache/hadoop/hbase/regionserver/

Author: nspiegelberg
Date: Tue Oct 11 19:12:56 2011
New Revision: 1182031

URL: http://svn.apache.org/viewvc?rev=1182031&view=rev
Log:
Compactions must fail if column tracker gets columns out of order

Summary:
We found this in ScanWildcardColumnTracker:

// new col < oldcol
// if (cmp < 0) {
// WARNING: This means that very likely an edit for some other family
// was incorrectly stored into the store for this one. Continue, but
// complain.
LOG.error("ScanWildcardColumnTracker.checkColumn ran " +
"into a column actually smaller than the previous column: " +

This went under the radar in our dark launch cluster when a column family name
was misspelled first, but then was "renamed" by renaming directories in the
HBase storage directory tree. We ended up with inconsistent data, but
compactions still succeeded most of the time, likely discarding part of input
data.

Test Plan: Run unit tests. 5-node cluster. Dark launch.

Reviewers: kannan, liyintang

Reviewed By: kannan

CC: hbase-eng@lists, kannan, nspiegelberg

Differential Revision: 334167

Revert Plan: OK

Modified:
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/KeyValue.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
    hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
    hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/KeyValue.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/KeyValue.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/KeyValue.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/KeyValue.java Tue Oct 11 19:12:56 2011
@@ -577,6 +577,17 @@ public class KeyValue implements Writabl
     return ret;
   }
 
+  /**
+   * Creates a shallow copy of this KeyValue, reusing the data byte buffer.
+   * http://en.wikipedia.org/wiki/Object_copy
+   * @return Shallow copy of this KeyValue
+   */
+  public KeyValue shallowCopy() {
+    KeyValue shallowCopy = new KeyValue(this.bytes, this.offset, this.length);
+    shallowCopy.setMemstoreTS(this.memstoreTS);
+    return shallowCopy;
+  }
+
   //---------------------------------------------------------------------------
   //
   //  String representation

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ColumnTracker.java Tue Oct 11 19:12:56 2011
@@ -19,6 +19,10 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import java.io.IOException;
+
+import org.apache.hadoop.hbase.regionserver.ScanQueryMatcher.MatchCode;
+
 /**
  * Implementing classes of this interface will be used for the tracking
  * and enforcement of columns and numbers of versions during the course of a
@@ -44,9 +48,11 @@ public interface ColumnTracker {
    * @param length
    * @param timestamp
    * @return The match code instance.
+   * @throws IOException in case there is an internal consistency problem
+   *      caused by a data corruption.
    */
   public ScanQueryMatcher.MatchCode checkColumn(byte [] bytes, int offset,
-      int length, long timestamp);
+      int length, long ttl) throws IOException;
 
   /**
    * Updates internal variables in between files

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java Tue Oct 11 19:12:56 2011
@@ -28,6 +28,7 @@ import org.apache.hadoop.hbase.filter.Fi
 import org.apache.hadoop.hbase.io.TimeRange;
 import org.apache.hadoop.hbase.util.Bytes;
 
+import java.io.IOException;
 import java.util.NavigableSet;
 
 /**
@@ -112,8 +113,10 @@ public class ScanQueryMatcher {
    *
    * @param kv KeyValue to check
    * @return The match code instance.
+   * @throws IOException in case there is an internal consistency problem
+   *      caused by a data corruption.
    */
-  public MatchCode match(KeyValue kv) {
+  public MatchCode match(KeyValue kv) throws IOException {
     if (filter != null && filter.filterAllRemaining()) {
       return MatchCode.DONE_SCAN;
     }

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ScanWildcardColumnTracker.java Tue Oct 11 19:12:56 2011
@@ -20,6 +20,8 @@
 
 package org.apache.hadoop.hbase.regionserver;
 
+import java.io.IOException;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.HConstants;
@@ -62,7 +64,7 @@ public class ScanWildcardColumnTracker i
    */
   @Override
   public MatchCode checkColumn(byte[] bytes, int offset, int length,
-      long timestamp) {
+      long timestamp) throws IOException {
     if (columnBuffer == null) {
       // first iteration.
       columnBuffer = bytes;
@@ -106,23 +108,13 @@ public class ScanWildcardColumnTracker i
     }
 
     // new col < oldcol
-    // if (cmp < 0) {
     // WARNING: This means that very likely an edit for some other family
-    // was incorrectly stored into the store for this one. Continue, but
-    // complain.
-    LOG.error("ScanWildcardColumnTracker.checkColumn ran " +
-  		"into a column actually smaller than the previous column: " +
-      Bytes.toStringBinary(bytes, offset, length));
-    // switched columns
-    columnBuffer = bytes;
-    columnOffset = offset;
-    columnLength = length;
-    currentCount = 0;
-    if (++currentCount > maxVersions) {
-      return ScanQueryMatcher.MatchCode.SEEK_NEXT_COL;
-    }
-    setTS(timestamp);
-    return ScanQueryMatcher.MatchCode.INCLUDE;
+    // was incorrectly stored into the store for this one. Throw an exception,
+    // because this might lead to data corruption.
+    throw new IOException(
+        "ScanWildcardColumnTracker.checkColumn ran into a column actually " +
+        "smaller than the previous column: " +
+        Bytes.toStringBinary(bytes, offset, length));
   }
 
   @Override

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Tue Oct 11 19:12:56 2011
@@ -1760,4 +1760,9 @@ public class Store implements HeapSize {
   public long heapSize() {
     return DEEP_OVERHEAD + this.memstore.heapSize();
   }
+
+  public KeyValue.KVComparator getComparator() {
+    return comparator;
+  }
+
 }

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java Tue Oct 11 19:12:56 2011
@@ -295,11 +295,25 @@ class StoreScanner extends NonLazyKeyVal
       matcher.setRow(peeked.getRow());
     }
     KeyValue kv;
+    KeyValue prevKV = null;
     List<KeyValue> results = new ArrayList<KeyValue>();
+
+    // Only do a sanity-check if store and comparator are available.
+    KeyValue.KVComparator comparator =
+        store != null ? store.getComparator() : null;
+
     LOOP: while((kv = this.heap.peek()) != null) {
       // kv is no longer immutable due to KeyOnlyFilter! use copy for safety
-      KeyValue copyKv = new KeyValue(kv.getBuffer(), kv.getOffset(), kv.getLength());
+      KeyValue copyKv = kv.shallowCopy();
+      // Check that the heap gives us KVs in an increasing order.
+      if (prevKV != null && comparator != null
+          && comparator.compare(prevKV, kv) > 0) {
+        throw new IOException("Key " + prevKV + " followed by a " +
+            "smaller key " + kv + " in cf " + store);
+      }
+      prevKV = copyKv;
       ScanQueryMatcher.MatchCode qcode = matcher.match(copyKv);
+
       switch(qcode) {
         case INCLUDE:
         case INCLUDE_AND_SEEK_NEXT_ROW:

Modified: hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java (original)
+++ hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java Tue Oct 11 19:12:56 2011
@@ -20,6 +20,7 @@
 
 package org.apache.hadoop.hbase.regionserver;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.TreeSet;
@@ -42,7 +43,7 @@ public class TestExplicitColumnTracker e
   private void runTest(int maxVersions,
                        TreeSet<byte[]> trackColumns,
                        List<byte[]> scannerColumns,
-                       List<MatchCode> expected) {
+                       List<MatchCode> expected) throws IOException {
     ColumnTracker exp = new ExplicitColumnTracker(
       trackColumns, maxVersions);
 
@@ -66,7 +67,7 @@ public class TestExplicitColumnTracker e
     }
   }
 
-  public void testGet_SingleVersion(){
+  public void testGet_SingleVersion() throws IOException{
     if(PRINT){
       System.out.println("SingleVersion");
     }
@@ -95,7 +96,7 @@ public class TestExplicitColumnTracker e
     runTest(maxVersions, columns, scanner, expected);
   }
 
-  public void testGet_MultiVersion(){
+  public void testGet_MultiVersion() throws IOException{
     if(PRINT){
       System.out.println("\nMultiVersion");
     }
@@ -154,7 +155,7 @@ public class TestExplicitColumnTracker e
   /**
    * hbase-2259
    */
-  public void testStackOverflow(){
+  public void testStackOverflow() throws IOException{
     int maxVersions = 1;
     TreeSet<byte[]> columns = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
     for (int i = 0; i < 100000; i++) {
@@ -177,7 +178,7 @@ public class TestExplicitColumnTracker e
   /**
    * Regression test for HBASE-2545
    */
-  public void testInfiniteLoop() {
+  public void testInfiniteLoop() throws IOException {
     TreeSet<byte[]> columns = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
     columns.addAll(Arrays.asList(new byte[][] {
       col2, col3, col5 }));

Modified: hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java?rev=1182031&r1=1182030&r2=1182031&view=diff
==============================================================================
--- hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java (original)
+++ hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanWildcardColumnTracker.java Tue Oct 11 19:12:56 2011
@@ -20,6 +20,7 @@
 
 package org.apache.hadoop.hbase.regionserver;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -31,7 +32,7 @@ public class TestScanWildcardColumnTrack
 
   final static int VERSIONS = 2;
 
-  public void testCheckColumn_Ok() {
+  public void testCheckColumn_Ok() throws IOException {
     ScanWildcardColumnTracker tracker =
       new ScanWildcardColumnTracker(VERSIONS);
 
@@ -63,7 +64,7 @@ public class TestScanWildcardColumnTrack
     }
   }
 
-  public void testCheckColumn_EnforceVersions() {
+  public void testCheckColumn_EnforceVersions() throws IOException {
     ScanWildcardColumnTracker tracker =
       new ScanWildcardColumnTracker(VERSIONS);