HBASE-532 Odd interaction between HRegion.get, HRegion.deleteAll and compactions

Found an issue in the 532 commit.  The MemcacheScanner had a flipped isWildcardScanner
test; fixing it returned wrong columns because using okCols rather than the literals
passed in.


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

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

@@ -131,10 +131,6 @@ public abstract class HAbstractScanner implements InternalScanner {
         this.wildcardMatch = true;
       }
       matchers.add(matcher);
-      // TODO: Does this multipleMatchers matter any more now that scanners
-      // are done at the store level?  It might have mattered when scanners
-      // could have been done at the region level when memcache was at the
-      // region level rather than down here at store level.
       if (matchers.size() > 1) {
         this.multipleMatchers = true;
       }
@@ -184,4 +180,4 @@ public abstract class HAbstractScanner implements InternalScanner {
     throw new UnsupportedOperationException("Unimplemented serverside. " +
       "next(HStoreKey, StortedMap(...) is more efficient");
   }
-}
+}

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

@@ -939,7 +939,6 @@ public class HRegion implements HConstants {
       return -1;
     }
     long startTime = System.currentTimeMillis();
-    
     if(LOG.isDebugEnabled()) {
       LOG.debug("Started memcache flush for region " +
         this.regionInfo.getRegionName() + ". Size " +
@@ -948,13 +947,11 @@ public class HRegion implements HConstants {
 
     // We reset the aggregate memcache size here so that subsequent updates
     // will add to the unflushed size
-    
     this.memcacheSize.set(0L);
     this.flushRequested = false;
     
     // Record latest flush time
     this.lastFlushTime = System.currentTimeMillis();
-    
     for (HStore hstore: stores.values()) {
       hstore.snapshotMemcache();
     }

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

@@ -590,8 +590,9 @@ public class HStore implements HConstants {
   //////////////////////////////////////////////////////////////////////////////
 
   /**
-   * Prior to doing a cache flush, we need to snapshot the memcache. Locking is
-   * handled by the memcache.
+   * Prior to doing a cache flush, we need to snapshot the memcache.
+   * TODO: This method is ugly.  Why let client of HStore run snapshots.  How
+   * do we know they'll be cleaned up?
    */
   void snapshotMemcache() {
     this.memcache.snapshot();

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

@@ -23,8 +23,10 @@ package org.apache.hadoop.hbase.regionserver;
 import java.io.IOException;
 import java.rmi.UnexpectedException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -75,7 +77,7 @@ class Memcache {
   }
 
   /**
-   * Creates a snapshot of the current Memcache
+   * Creates a snapshot of the current Memcache or returns existing snapshot.
    * Must be followed by a call to {@link #clearSnapshot(SortedMap)}
    * @return Snapshot. Never null.  May have no entries.
    */
@@ -84,8 +86,9 @@ class Memcache {
     try {
       // If snapshot has entries, then flusher failed or didn't call cleanup.
       if (this.snapshot.size() > 0) {
-        LOG.warn("Returning extant snapshot. Is there another ongoing " +
-          "flush or did last attempt fail?");
+        LOG.debug("Returning existing snapshot. Either the snapshot was run " +
+          "by the region -- normal operation but to be fixed -- or there is " +
+          "another ongoing flush or did we fail last attempt?");
         return this.snapshot;
       }
       // We used to synchronize on the memcache here but we're inside a
@@ -236,10 +239,10 @@ class Memcache {
   /**
    * Return all the available columns for the given key.  The key indicates a 
    * row and timestamp, but not a column name.
-   *
-   * The returned object should map column names to byte arrays (byte[]).
    * @param key
-   * @param results
+   * @param columns Pass null for all columns else the wanted subset.
+   * @param deletes Map to accumulate deletes found.
+   * @param results Where to stick row results found.
    */
   void getFull(HStoreKey key, Set<Text> columns, Map<Text, Long> deletes, 
     Map<Text, Cell> results) {
@@ -565,7 +568,7 @@ class Memcache {
 
   private class MemcacheScanner extends HAbstractScanner {
     private Text currentRow;
-    private final Set<Text> columns;
+    private Set<Text> columns = null;
     
     MemcacheScanner(final long timestamp, final Text targetCols[],
       final Text firstRow)
@@ -577,7 +580,13 @@ class Memcache {
       // If we're being asked to scan explicit columns rather than all in 
       // a family or columns that match regexes, cache the sorted array of
       // columns.
-      this.columns = this.isWildcardScanner()? this.okCols.keySet(): null;
+      this.columns = null;
+      if (!isWildcardScanner()) {
+        this.columns = new HashSet<Text>();
+        for (int i = 0; i < targetCols.length; i++) {
+          this.columns.add(targetCols[i]);
+        }
+      }
     }
 
     @Override
@@ -627,4 +636,4 @@ class Memcache {
       }
     }
   }
-}
+}