HBASE-1781 Weird behavior of WildcardColumnTracker.checkColumn(),

looks like recursive loop


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

CHANGES.txt

@@ -92,6 +92,8 @@ Release 0.21.0 - Unreleased
    HBASE-1919  code: HRS.delete seems to ignore exceptions it shouldnt
    HBASE-1951  Stack overflow when calling HTable.checkAndPut() 
                when deleting a lot of values
+   HBASE-1781  Weird behavior of WildcardColumnTracker.checkColumn(), 
+               looks like recursive loop
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

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

@@ -89,94 +89,57 @@ public class WildcardColumnTracker implements ColumnTracker {
    * @return MatchCode telling QueryMatcher what action to take
    */
   public MatchCode checkColumn(byte [] bytes, int offset, int length) {
+    boolean recursive = false;
+    do {
 
-    // Nothing to match against, add to new and include
+      // Nothing to match against, add to new and include
-    if(this.column == null && this.newColumn == null) {
+      if(this.column == null && this.newColumn == null) {
-      newColumns.add(new ColumnCount(bytes, offset, length, 1));
+        newColumns.add(new ColumnCount(bytes, offset, length, 1));
-      this.newColumn = newColumns.get(newIndex);
-      return MatchCode.INCLUDE;
-    }
-    
-    // Nothing old, compare against new
-    if(this.column == null && this.newColumn != null) {
-      int ret = Bytes.compareTo(newColumn.getBuffer(), newColumn.getOffset(), 
-          newColumn.getLength(), bytes, offset, length);
-      
-      // Same column
-      if(ret == 0) {
-        if(newColumn.increment() > this.maxVersions) {
-          return MatchCode.SKIP;
-        }
-        return MatchCode.INCLUDE;
-      }
-      
-      // Specified column is bigger than current column
-      // Move down current column and check again
-      if(ret <= -1) {
-        if(++newIndex == newColumns.size()) {
-          // No more, add to end and include
-          newColumns.add(new ColumnCount(bytes, offset, length, 1));
-          this.newColumn = newColumns.get(newIndex);
-          return MatchCode.INCLUDE;
-        }
         this.newColumn = newColumns.get(newIndex);
-        return checkColumn(bytes, offset, length);
-      }
-      
-      // ret >= 1
-      // Specified column is smaller than current column
-      // Nothing to match against, add to new and include
-      newColumns.add(new ColumnCount(bytes, offset, length, 1));
-      this.newColumn = newColumns.get(++newIndex);
-      return MatchCode.INCLUDE;
-    }
-    
-    // Nothing new, compare against old
-    if(this.newColumn == null && this.column != null) {
-      int ret = Bytes.compareTo(column.getBuffer(), column.getOffset(), 
-          column.getLength(), bytes, offset, length);
-      
-      // Same column
-      if(ret == 0) {
-        if(column.increment() > this.maxVersions) {
-          return MatchCode.SKIP;
-        }
         return MatchCode.INCLUDE;
       }
-      
+
-      // Specified column is bigger than current column
+      // Nothing old, compare against new
-      // Move down current column and check again
+      if(this.column == null && this.newColumn != null) {
-      if(ret <= -1) {
+        int ret = Bytes.compareTo(newColumn.getBuffer(), newColumn.getOffset(),
-        if(++index == columns.size()) {
+            newColumn.getLength(), bytes, offset, length);
-          // No more, add to new and include (new was empty prior to this)
+
-          newColumns.add(new ColumnCount(bytes, offset, length, 1));
+        // Same column
-          this.newColumn = newColumns.get(newIndex);
+        if(ret == 0) {
-          this.column = null;
+          if(newColumn.increment() > this.maxVersions) {
+            return MatchCode.SKIP;
+          }
           return MatchCode.INCLUDE;
         }
-        this.column = columns.get(index);
-        return checkColumn(bytes, offset, length);
-      }
-      
-      // ret >= 1
-      // Specified column is smaller than current column
-      // Nothing to match against, add to new and include
-      newColumns.add(new ColumnCount(bytes, offset, length, 1));
-      this.newColumn = newColumns.get(newIndex);
-      return MatchCode.INCLUDE;
-    }
 
-    if (column != null && newColumn != null) {
+        // Specified column is bigger than current column
-      // There are new and old, figure which to check first
+        // Move down current column and check again
-      int ret = Bytes.compareTo(column.getBuffer(), column.getOffset(), 
+        if(ret <= -1) {
-        column.getLength(), newColumn.getBuffer(), newColumn.getOffset(), 
+          if(++newIndex == newColumns.size()) {
-        newColumn.getLength());
+            // No more, add to end and include
-        
+            newColumns.add(new ColumnCount(bytes, offset, length, 1));
-      // Old is smaller than new, compare against old
+            this.newColumn = newColumns.get(newIndex);
-      if(ret <= -1) {
+            return MatchCode.INCLUDE;
-        ret = Bytes.compareTo(column.getBuffer(), column.getOffset(), 
+          }
-          column.getLength(), bytes, offset, length);
+          this.newColumn = newColumns.get(newIndex);
-      
+          //return checkColumn(bytes, offset, length);
+          recursive = true;
+          continue;
+        }
+
+        // ret >= 1
+        // Specified column is smaller than current column
+        // Nothing to match against, add to new and include
+        newColumns.add(new ColumnCount(bytes, offset, length, 1));
+        this.newColumn = newColumns.get(++newIndex);
+        return MatchCode.INCLUDE;
+      }
+
+      // Nothing new, compare against old
+      if(this.newColumn == null && this.column != null) {
+        int ret = Bytes.compareTo(column.getBuffer(), column.getOffset(),
+            column.getLength(), bytes, offset, length);
+
         // Same column
         if(ret == 0) {
           if(column.increment() > this.maxVersions) {
@@ -184,57 +147,105 @@ public class WildcardColumnTracker implements ColumnTracker {
           }
           return MatchCode.INCLUDE;
         }
-      
+
         // Specified column is bigger than current column
         // Move down current column and check again
         if(ret <= -1) {
           if(++index == columns.size()) {
+            // No more, add to new and include (new was empty prior to this)
+            newColumns.add(new ColumnCount(bytes, offset, length, 1));
+            this.newColumn = newColumns.get(newIndex);
             this.column = null;
-          } else {
+            return MatchCode.INCLUDE;
-            this.column = columns.get(index);
           }
-          return checkColumn(bytes, offset, length);
+          this.column = columns.get(index);
+          //return checkColumn(bytes, offset, length);
+          recursive = true;
+          continue;
         }
-      
+
+        // ret >= 1
+        // Specified column is smaller than current column
+        // Nothing to match against, add to new and include
+        newColumns.add(new ColumnCount(bytes, offset, length, 1));
+        this.newColumn = newColumns.get(newIndex);
+        return MatchCode.INCLUDE;
+      }
+
+      if (column != null && newColumn != null) {
+        // There are new and old, figure which to check first
+        int ret = Bytes.compareTo(column.getBuffer(), column.getOffset(),
+          column.getLength(), newColumn.getBuffer(), newColumn.getOffset(),
+          newColumn.getLength());
+
+        // Old is smaller than new, compare against old
+        if(ret <= -1) {
+          ret = Bytes.compareTo(column.getBuffer(), column.getOffset(),
+            column.getLength(), bytes, offset, length);
+
+          // Same column
+          if(ret == 0) {
+            if(column.increment() > this.maxVersions) {
+              return MatchCode.SKIP;
+            }
+            return MatchCode.INCLUDE;
+          }
+
+          // Specified column is bigger than current column
+          // Move down current column and check again
+          if(ret <= -1) {
+            if(++index == columns.size()) {
+              this.column = null;
+            } else {
+              this.column = columns.get(index);
+            }
+            //return checkColumn(bytes, offset, length);
+            recursive = true;
+            continue;
+          }
+
+          // ret >= 1
+          // Specified column is smaller than current column
+          // Nothing to match against, add to new and include
+          newColumns.add(new ColumnCount(bytes, offset, length, 1));
+          return MatchCode.INCLUDE;
+        }
+      }
+
+      if (newColumn != null) {
+        // Cannot be equal, so ret >= 1
+        // New is smaller than old, compare against new
+        int ret = Bytes.compareTo(newColumn.getBuffer(), newColumn.getOffset(),
+          newColumn.getLength(), bytes, offset, length);
+
+        // Same column
+        if(ret == 0) {
+          if(newColumn.increment() > this.maxVersions) {
+            return MatchCode.SKIP;
+          }
+          return MatchCode.INCLUDE;
+        }
+
+        // Specified column is bigger than current column
+        // Move down current column and check again
+        if(ret <= -1) {
+          if(++newIndex == newColumns.size()) {
+            this.newColumn = null;
+          } else {
+            this.newColumn = newColumns.get(newIndex);
+          }
+          //return checkColumn(bytes, offset, length);
+          recursive = true;
+          continue;
+        }
+
         // ret >= 1
         // Specified column is smaller than current column
         // Nothing to match against, add to new and include
         newColumns.add(new ColumnCount(bytes, offset, length, 1));
         return MatchCode.INCLUDE;
       }
-    }
+    } while(recursive);
-
-    if (newColumn != null) {
-      // Cannot be equal, so ret >= 1
-      // New is smaller than old, compare against new
-      int ret = Bytes.compareTo(newColumn.getBuffer(), newColumn.getOffset(), 
-        newColumn.getLength(), bytes, offset, length);
-    
-      // Same column
-      if(ret == 0) {
-        if(newColumn.increment() > this.maxVersions) {
-          return MatchCode.SKIP;
-        }
-        return MatchCode.INCLUDE;
-      }
-    
-      // Specified column is bigger than current column
-      // Move down current column and check again
-      if(ret <= -1) {
-        if(++newIndex == newColumns.size()) {
-          this.newColumn = null;
-        } else {
-          this.newColumn = newColumns.get(newIndex);
-        }
-        return checkColumn(bytes, offset, length);
-      }
-    
-      // ret >= 1
-      // Specified column is smaller than current column
-      // Nothing to match against, add to new and include
-      newColumns.add(new ColumnCount(bytes, offset, length, 1));
-      return MatchCode.INCLUDE;
-    }
 
     // No match happened, add to new and include
     newColumns.add(new ColumnCount(bytes, offset, length, 1));

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

@@ -329,6 +329,23 @@ implements HConstants {
       }
     }
   }
+
+  // HBASE-1781
+  public void testStackOverflow(){
+    int maxVersions = 1;
+
+    ColumnTracker wild = new WildcardColumnTracker(maxVersions);
+    for(int i = 0; i < 100000; i+=2) {
+      byte [] col = Bytes.toBytes("col"+i);
+      wild.checkColumn(col, 0, col.length);
+    }
+    wild.update();
+
+    for(int i = 1; i < 100000; i+=2) {
+      byte [] col = Bytes.toBytes("col"+i);
+      wild.checkColumn(col, 0, col.length);
+    }
+  }