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 19:45:34 UTC

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

Author: nspiegelberg
Date: Tue Oct 11 17:45:33 2011
New Revision: 1181982

URL: http://svn.apache.org/viewvc?rev=1181982&view=rev
Log:
HBASE-4433: avoid extra next (potentially a seek) if done with column/row

Summary:
When we are done with the requested column(s) the code still does an extra
next() call before it realizes that it is actually done. This extra next() call
could potentially result in an unnecessary extra block load. This is likely to
be especially bad for CFs where the KVs are large blobs (such as
Snapshot/ThreadSnapshot CF) where each KV may be occupying a block of its own.
So the next() can often load a new unrelated block unnecessarily.

Test Plan: running unit tests right now; writing a custom test to count number
of blocks accessed with blockcache size set to pretty small; will do dev testing
etc.

Reviewers: mbautin, kranganathan, liyintang, nspiegelberg

Reviewed By: kranganathan

CC: hbase-eng@lists, hbase@lists, kannan, mbautin, nspiegelberg, kranganathan

Differential Revision: 327923

Modified:
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.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/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/TestQueryMatcher.java

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java?rev=1181982&r1=1181981&r2=1181982&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java Tue Oct 11 17:45:33 2011
@@ -48,6 +48,12 @@ import org.apache.hadoop.hbase.util.Byte
 public class ExplicitColumnTracker implements ColumnTracker {
 
   private final int maxVersions;
+
+  /**
+   * Contains the list of columns that the ExplicitColumnTracker is tracking.
+   * Each ColumnCount instance also tracks how many versions of the requested
+   * column have been returned.
+   */
   private final List<ColumnCount> columns;
   private final List<ColumnCount> columnsToReuse;
   private int index;
@@ -116,15 +122,25 @@ public class ExplicitColumnTracker imple
           //If duplicate, skip this Key
           return ScanQueryMatcher.MatchCode.SKIP;
         }
-        if(this.column.decrement() == 0) {
+        if (this.column.decrement() == 0) {
+
           // Done with versions for this column
+          // Note: because we are done with this column, and are removing
+          // it from columns, we don't do a ++this.index. The index stays
+          // the same but the columns have shifted within the array such
+          // that index now points to the next column we are interested in.
           this.columns.remove(this.index);
           resetTS();
-          if(this.columns.size() == this.index) {
-            // Will not hit any more columns in this storefile
+
+          if (this.columns.size() == this.index) {
+            // We have served all the request columns.
             this.column = null;
+            return ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW;
           } else {
+            // We are done with current column; advance to next column
+            // of interest.
             this.column = this.columns.get(this.index);
+            return ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL;
           }
         } else {
           setTS(timestamp);
@@ -135,15 +151,18 @@ public class ExplicitColumnTracker imple
       resetTS();
 
       if (ret > 0) {
-        // Specified column is smaller than the current, skip to next column.
+        // The current KV is smaller than the column the ExplicitColumnTracker
+        // is interested in, so seek to that column of interest.
         return ScanQueryMatcher.MatchCode.SEEK_NEXT_COL;
       }
 
-      // Specified column is bigger than current column
-      // Move down current column and check again
-      if(ret <= -1) {
-        if(++this.index >= this.columns.size()) {
-          // No more to match, do not include, done with storefile
+      // The current KV is bigger than the column the ExplicitColumnTracker
+      // is interested in. That means there is no more data for the column
+      // of interest. Advance the ExplicitColumnTracker state to next
+      // column of interest, and check again.
+      if (ret <= -1) {
+        if (++this.index >= this.columns.size()) {
+          // No more to match, do not include, done with this row.
           return ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW; // done_row
         }
         // This is the recursive case.

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=1181982&r1=1181981&r2=1181982&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 17:45:33 2011
@@ -387,5 +387,15 @@ public class ScanQueryMatcher {
      * Seek to next key which is given as hint.
      */
     SEEK_NEXT_USING_HINT,
+
+    /**
+     * Include KeyValue and done with column, seek to next.
+     */
+    INCLUDE_AND_SEEK_NEXT_COL,
+
+    /**
+     * Include KeyValue and done with row, seek to next.
+     */
+    INCLUDE_AND_SEEK_NEXT_ROW,
   }
 }

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=1181982&r1=1181981&r2=1181982&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 17:45:33 2011
@@ -294,6 +294,8 @@ class StoreScanner implements KeyValueSc
       //DebugPrint.println("SS peek kv = " + kv + " with qcode = " + qcode);
       switch(qcode) {
         case INCLUDE:
+        case INCLUDE_AND_SEEK_NEXT_ROW:
+        case INCLUDE_AND_SEEK_NEXT_COL:
           this.countPerRow++;
           if (storeLimit > 0 && this.countPerRow > storeLimit) {
             // do what SEEK_NEXT_ROW does.
@@ -307,7 +309,19 @@ class StoreScanner implements KeyValueSc
 
           HRegion.incrNumericMetric(this.metricNameGetsize, copyKv.getLength());
           results.add(copyKv);
-          this.heap.next();
+
+          if (qcode == ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW) {
+            if (!matcher.moreRowsMayExistAfter(kv)) {
+              outResult.addAll(results);
+              return false;
+            }
+            reseek(matcher.getKeyForNextRow(kv));
+          } else if (qcode == ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL) {
+            reseek(matcher.getKeyForNextColumn(kv));
+          } else {
+            this.heap.next();
+          }
+
           if (limit > 0 && (results.size() == limit)) {
             break LOOP;
           }

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=1181982&r1=1181981&r2=1181982&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 17:45:33 2011
@@ -77,11 +77,11 @@ public class TestExplicitColumnTracker e
     columns.add(col2);
     columns.add(col4);
     List<MatchCode> expected = new ArrayList<ScanQueryMatcher.MatchCode>();
-    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
-    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
-    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW);
+    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);             // col1
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL); // col2
+    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);             // col3
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW); // col4
+    expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW);             // col5
     int maxVersions = 1;
 
     //Create "Scanner"
@@ -111,16 +111,16 @@ public class TestExplicitColumnTracker e
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
 
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);                   // col2; 1st version
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL); // col2; 2nd version
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
 
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
 
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);                   // col4; 1st version
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW); // col4; 2nd version
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW);
 
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW);

Modified: hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java?rev=1181982&r1=1181981&r2=1181982&view=diff
==============================================================================
--- hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java (original)
+++ hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java Tue Oct 11 17:45:33 2011
@@ -88,10 +88,10 @@ public class TestQueryMatcher extends HB
     //Expected result
     List<MatchCode> expected = new ArrayList<ScanQueryMatcher.MatchCode>();
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL);
     expected.add(ScanQueryMatcher.MatchCode.SEEK_NEXT_COL);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
-    expected.add(ScanQueryMatcher.MatchCode.INCLUDE);
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL);
+    expected.add(ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW);
     expected.add(ScanQueryMatcher.MatchCode.DONE);
 
     // 2,4,5
@@ -182,9 +182,9 @@ public class TestQueryMatcher extends HB
     long testTTL = 1000;
     MatchCode [] expected = new MatchCode[] {
         ScanQueryMatcher.MatchCode.SEEK_NEXT_COL,
-        ScanQueryMatcher.MatchCode.INCLUDE,
+        ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL,
         ScanQueryMatcher.MatchCode.SEEK_NEXT_COL,
-        ScanQueryMatcher.MatchCode.INCLUDE,
+        ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_COL,
         ScanQueryMatcher.MatchCode.SEEK_NEXT_ROW,
         ScanQueryMatcher.MatchCode.DONE
     };