You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ns...@apache.org on 2011/10/11 04:09:48 UTC
svn commit: r1181455 - in /hbase/branches/0.89/src:
main/java/org/apache/hadoop/hbase/regionserver/Store.java
test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java
Author: nspiegelberg
Date: Tue Oct 11 02:09:45 2011
New Revision: 1181455
URL: http://svn.apache.org/viewvc?rev=1181455&view=rev
Log:
Compaction Aglorithm Mods after Peer Review
Summary:
Kannan had a couple extra suggestions after the initial peer
review that were bugs/limitations. Fixing all those issues and adding
associated unit tests
Test Plan:
- mvn test -Dtest=TestCompactSelection
- mvn test (underway)
DiffCamp Revision: 188610
Reviewed By: kannan
Reviewers: jgray, kannan
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=1181455&r1=1181454&r2=1181455&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:09:45 2011
@@ -94,7 +94,7 @@ public class Store implements HeapSize {
final Configuration conf;
// ttl in milliseconds.
protected long ttl;
- private long majorCompactionTime;
+ long majorCompactionTime;
private final int minFilesToCompact;
private final int maxFilesToCompact;
private final long minCompactSize;
@@ -198,7 +198,7 @@ public class Store implements HeapSize {
this.minCompactSize = conf.getLong("hbase.hstore.compaction.min.size",
this.region.memstoreFlushSize);
this.maxCompactSize
- = conf.getLong("hbase.hstore.compaction.max.size", 0);
+ = conf.getLong("hbase.hstore.compaction.max.size", Long.MAX_VALUE);
this.compactRatio = conf.getFloat("hbase.hstore.compaction.ratio", 1.2F);
if (Store.closeCheckInterval == 0) {
@@ -818,23 +818,19 @@ public class Store implements HeapSize {
*/
List<StoreFile> filesToCompact = new ArrayList<StoreFile>(candidates);
- // do not compact files above a configurable max filesize
- // save all references. we MUST compact them
- if (this.maxCompactSize > 0) {
- final long msize = this.maxCompactSize;
- filesToCompact.removeAll(Collections2.filter(filesToCompact,
- new Predicate<StoreFile>() {
- public boolean apply(StoreFile sf) {
- // NOTE: keep all references. we must compact them
- return sf.getReader().length() > msize && !sf.isReference();
- }
- }));
+ if (!forcemajor) {
+ // do not compact old files above a configurable threshold
+ // save all references. we MUST compact them
+ int pos = 0;
+ while (pos < filesToCompact.size() &&
+ filesToCompact.get(pos).getReader().length() > maxCompactSize &&
+ !filesToCompact.get(pos).isReference()) ++pos;
+ filesToCompact.subList(0, pos).clear();
}
// major compact on user action or age (caveat: we have too many files)
- boolean majorcompaction = forcemajor ||
- (!filesToCompact.isEmpty() && isMajorCompaction(filesToCompact)
- && filesToCompact.size() > this.maxFilesToCompact);
+ boolean majorcompaction = (forcemajor || isMajorCompaction(filesToCompact))
+ && filesToCompact.size() < this.maxFilesToCompact;
if (filesToCompact.isEmpty()) {
LOG.debug(this.storeNameStr + ": no store files to compact");
@@ -846,8 +842,10 @@ public class Store implements HeapSize {
int start = 0;
double r = this.compactRatio;
+ /* TODO: add sorting + unit test back in when HBASE-2856 is fixed
//sort files by size to correct when normal skew is altered by bulk load
Collections.sort(filesToCompact, StoreFile.Comparators.FILE_SIZE);
+ */
// get store file sizes for incremental compacting selection.
int countOfFiles = filesToCompact.size();
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=1181455&r1=1181454&r2=1181455&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:09:45 2011
@@ -139,20 +139,24 @@ public class TestCompactSelection extend
return ret;
}
- void compactEquals(List<StoreFile> actual, long ... expected)
+ long[] getSizes(List<StoreFile> sfList) {
+ long[] aNums = new long[sfList.size()];
+ for (int i=0; i <sfList.size(); ++i) {
+ aNums[i] = sfList.get(i).getReader().length();
+ }
+ return aNums;
+ }
+
+ void compactEquals(List<StoreFile> candidates, long ... expected)
throws IOException {
- compactEquals(actual, false, expected);
+ compactEquals(candidates, false, expected);
}
- void compactEquals(List<StoreFile> actual, boolean forcemajor,
+ void compactEquals(List<StoreFile> candidates, boolean forcemajor,
long ... expected)
throws IOException {
- List<StoreFile> result = store.compactSelection(actual, forcemajor);
- long[] aNums = new long[result.size()];
- for (int i=0; i <result.size(); ++i) {
- aNums[i] = result.get(i).getReader().length();
- }
- assertEquals(Arrays.toString(expected), Arrays.toString(aNums));
+ List<StoreFile> actual = store.compactSelection(candidates, forcemajor);
+ assertEquals(Arrays.toString(expected), Arrays.toString(getSizes(actual)));
}
public void testCompactionRatio() throws IOException {
@@ -173,32 +177,44 @@ public class TestCompactSelection extend
compactEquals(sfCreate(tooBig, tooBig, 700,700) /* empty */);
// small files = don't care about ratio
compactEquals(sfCreate(8,3,1), 8,3,1);
+ /* TODO: add sorting + unit test back in when HBASE-2856 is fixed
// sort first so you don't include huge file the tail end.
// happens with HFileOutputFormat bulk migration
compactEquals(sfCreate(100,50,23,12,12, 500), 23, 12, 12);
+ */
// don't exceed max file compact threshold
assertEquals(maxFiles,
store.compactSelection(sfCreate(7,6,5,4,3,2,1), false).size());
/* MAJOR COMPACTION */
// if a major compaction has been forced, then compact everything
- compactEquals(sfCreate(100,50,25,12,12), true, 100, 50, 25, 12, 12);
+ compactEquals(sfCreate(50,25,12,12), true, 50, 25, 12, 12);
// also choose files < threshold on major compaction
compactEquals(sfCreate(12,12), true, 12, 12);
- // unless one of those files is too big
- compactEquals(sfCreate(tooBig, 12,12), true, 12, 12);
+ // even if one of those files is too big
+ compactEquals(sfCreate(tooBig, 12,12), true, tooBig, 12, 12);
// don't exceed max file compact threshold, even with major compaction
assertEquals(maxFiles,
store.compactSelection(sfCreate(7,6,5,4,3,2,1), true).size());
+ // if we exceed maxCompactSize, downgrade to minor
+ // if not, it creates a 'snowball effect' when files >> maxCompactSize:
+ // the last file in compaction is the aggregate of all previous compactions
+ compactEquals(sfCreate(100,50,23,12,12), true, 23, 12, 12);
+ // trigger an aged major compaction
+ store.majorCompactionTime = 1;
+ compactEquals(sfCreate(50,25,12,12), 50, 25, 12, 12);
+ // major sure exceeding maxCompactSize also downgrades aged minors
+ store.majorCompactionTime = 1;
+ compactEquals(sfCreate(100,50,23,12,12), 23, 12, 12);
/* REFERENCES == file is from a region that was split */
// treat storefiles that have references like a major compaction
- compactEquals(sfCreate(true, 100,50,25,12,12), true, 100, 50, 25, 12, 12);
+ compactEquals(sfCreate(true, 100,50,25,12,12), 100, 50, 25, 12, 12);
// reference files shouldn't obey max threshold
- compactEquals(sfCreate(true, tooBig, 12,12), true, tooBig, 12, 12);
+ compactEquals(sfCreate(true, tooBig, 12,12), tooBig, 12, 12);
// reference files should obey max file compact to avoid OOM
assertEquals(maxFiles,
- store.compactSelection(sfCreate(true, 7,6,5,4,3,2,1), true).size());
+ store.compactSelection(sfCreate(true, 7,6,5,4,3,2,1), false).size());
// empty case
compactEquals(new ArrayList<StoreFile>() /* empty */);