svn commit: r1181464 - in /hbase/branches/0.89/src: main/java/org/apache/hadoop/hbase/regionserver/Store.java test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java

[ns...@apache.org]

Author: nspiegelberg
Date: Tue Oct 11 02:11:00 2011
New Revision: 1181464

URL: http://svn.apache.org/viewvc?rev=1181464&view=rev
Log:
Fix MajorCompactionChecker

Summary:
Fixed 2 problems with the MCC thread, which runs ~3hr
1: exclude StoreFiles past the max compact size (which will be excluded
in the selection process)
2: don't reset the majorCompactTime until a major compaction occurs
(since there might be jitter)

Test Plan:
- mvn test (in progress)

DiffCamp Revision: 193591
Reviewed By: kannan
Reviewers: jgray, kannan, kranganathan
CC: nspiegelberg, kannan
Revert Plan:
OK

Modified:
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
    hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1181464&r1=1181463&r2=1181464&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Tue Oct 11 02:11:00 2011
@@ -731,7 +731,24 @@ public class Store implements HeapSize {
    * @return True if we should run a major compaction.
    */
   boolean isMajorCompaction() throws IOException {
-    return isMajorCompaction(storefiles);
+    for (StoreFile sf : this.storefiles) {
+      if (sf.getReader() == null) {
+        LOG.debug("StoreFile " + sf + " has null Reader");
+        return false;
+      }
+    }
+
+    List<StoreFile> candidates = new ArrayList<StoreFile>(this.storefiles);
+
+    // exclude files above the max compaction threshold
+    // except: save all references. we MUST compact them
+    int pos = 0;
+    while (pos < candidates.size() &&
+           candidates.get(pos).getReader().length() > this.maxCompactSize &&
+           !candidates.get(pos).isReference()) ++pos;
+    candidates.subList(0, pos).clear();
+
+    return isMajorCompaction(candidates);
   }
 
   /*
@@ -764,7 +781,6 @@ public class Store implements HeapSize {
             StringUtils.formatTimeDiff(now, lowTimestamp));
         }
         result = true;
-        this.majorCompactionTime = getNextMajorCompactTime();
       }
     }
     return result;
@@ -836,15 +852,18 @@ public class Store implements HeapSize {
       filesToCompact.subList(0, pos).clear();
     }
 
-    // major compact on user action or age (caveat: we have too many files)
-    boolean majorcompaction = (forcemajor || isMajorCompaction(filesToCompact))
-      && filesToCompact.size() < this.maxFilesToCompact;
-
     if (filesToCompact.isEmpty()) {
       LOG.debug(this.storeNameStr + ": no store files to compact");
       return filesToCompact;
     }
 
+    // major compact on user action or age (caveat: we have too many files)
+    boolean majorcompaction = (forcemajor || isMajorCompaction(filesToCompact))
+      && filesToCompact.size() < this.maxFilesToCompact;
+    if (majorcompaction) {
+      this.majorCompactionTime = getNextMajorCompactTime();
+    }
+
     if (!majorcompaction && !hasReferences(filesToCompact)) {
       // we're doing a minor compaction, let's see what files are applicable
       int start = 0;

Modified: hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java?rev=1181464&r1=1181463&r2=1181464&view=diff
==============================================================================
--- hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java (original)
+++ hbase/branches/0.89/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java Tue Oct 11 02:11:00 2011
@@ -51,6 +51,7 @@ public class TestCompactSelection extend
   private Store store;
   private static final String DIR
     = HBaseTestingUtility.getTestDir() + "/TestCompactSelection/";
+  private static Path TEST_FILE;
 
   private static final int minFiles = 3;
   private static final int maxFiles = 5;
@@ -86,6 +87,8 @@ public class TestCompactSelection extend
     HRegion region = new HRegion(basedir, hlog, fs, conf, info, null);
 
     store = new Store(basedir, region, hcd, fs, conf);
+    TEST_FILE = StoreFile.getRandomFilename(fs, store.getHomedir());
+    fs.create(TEST_FILE);
   }
 
   // used so our tests don't deal with actual StoreFiles
@@ -94,7 +97,7 @@ public class TestCompactSelection extend
     boolean isRef = false;
 
     MockStoreFile(long length, boolean isRef) throws IOException {
-      super(TEST_UTIL.getTestFileSystem(), new Path("_"), false,
+      super(TEST_UTIL.getTestFileSystem(), TEST_FILE, false,
             TEST_UTIL.getConfiguration(), BloomType.NONE, false);
       this.length = length;
       this.isRef  = isRef;