diff --git a/src/java/org/apache/hadoop/hbase/regionserver/HAbstractScanner.java b/src/java/org/apache/hadoop/hbase/regionserver/HAbstractScanner.java index 7c0fe671a15..8cd0ee7e691 100644 --- a/src/java/org/apache/hadoop/hbase/regionserver/HAbstractScanner.java +++ b/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"); } -} \ No newline at end of file +} diff --git a/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 83af23c3b77..74e633ed363 100644 --- a/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/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(); } diff --git a/src/java/org/apache/hadoop/hbase/regionserver/HStore.java b/src/java/org/apache/hadoop/hbase/regionserver/HStore.java index ba5d53cd8f9..587ec099016 100644 --- a/src/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/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(); diff --git a/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java b/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java index f66c6b89c8e..a40572fab2f 100644 --- a/src/java/org/apache/hadoop/hbase/regionserver/Memcache.java +++ b/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 columns, Map deletes, Map results) { @@ -565,7 +568,7 @@ class Memcache { private class MemcacheScanner extends HAbstractScanner { private Text currentRow; - private final Set columns; + private Set 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(); + for (int i = 0; i < targetCols.length; i++) { + this.columns.add(targetCols[i]); + } + } } @Override @@ -627,4 +636,4 @@ class Memcache { } } } -} \ No newline at end of file +}