svn commit: r1181606 - in /hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver: HRegion.java MemStoreFlusher.java

[ns...@apache.org]

Author: nspiegelberg
Date: Tue Oct 11 02:25:15 2011
New Revision: 1181606

URL: http://svn.apache.org/viewvc?rev=1181606&view=rev
Log:
fix TestHRegion:testBasicSplit() unit test failure

Summary:
compactStores() wasn't compacting all stores in the region if a split had been
requested for the region. After compacting the first store, it would notice that
a split had been requested, and then return with the midkey without compacting
the remaining stores.

This isn't safe if there are reference (half file references) to parent regions
in some of the column families.

Also fixed flush code path. If a split has been requested, but there are half
file references in the region, we still need to compact.

Test Plan: Unit tests. TestHRegion now passes. Running all unit tests.
Reviewed By: nspiegelberg
Reviewers: mbautin, nspiegelberg, kranganathan
CC: hbase@lists, nspiegelberg
Differential Revision: 282246

Modified:
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1181606&r1=1181605&r2=1181606&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Tue Oct 11 02:25:15 2011
@@ -972,7 +972,9 @@ public class HRegion implements HeapSize
    * to be split.
    */
   public byte[] compactStores() throws IOException {
-    for(Store s : getStores().values()) {
+
+    // first compact all stores
+    for (Store s : getStores().values()) {
       CompactionRequest cr = s.requestCompaction();
       if(cr != null) {
         try {
@@ -981,6 +983,15 @@ public class HRegion implements HeapSize
           s.finishRequest(cr);
         }
       }
+    }
+
+    // if for some reason we still have references, we can't split further
+    if (hasReferences()) {
+      return null;
+    }
+
+    // check if we need to split now; and if so find the midkey
+    for (Store s : getStores().values()) {
       byte[] splitRow = s.checkSplit();
       if (splitRow != null) {
         return splitRow;

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java?rev=1181606&r1=1181605&r2=1181606&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java Tue Oct 11 02:25:15 2011
@@ -212,7 +212,16 @@ class MemStoreFlusher extends Thread imp
           LOG.warn("Region " + region.getRegionNameAsString() + " has too many " +
             "store files; delaying flush up to " + this.blockingWaitTime + "ms");
         }
-        if (!this.server.compactSplitThread.requestSplit(region)) {
+
+        /* If a split has been requested, we avoid scheduling a compaction
+         * request because we'll have to compact after the split anyway.
+         * However, if the region has reference files (from a previous split),
+         * we do need to let the compactions go through so that half file
+         * references to parent regions are removed, and we can split this
+         * region further.
+         */
+        if (!this.server.compactSplitThread.requestSplit(region)
+            || region.hasReferences()) {
           this.server.compactSplitThread.requestCompaction(region, getName());
         }
         // Put back on the queue.  Have it come back out of the queue