From ecb4b1017c23086bbc3bd422502ede8350e2c90c Mon Sep 17 00:00:00 2001 From: sershe Date: Fri, 8 Nov 2013 23:17:51 +0000 Subject: [PATCH] HBASE-9921 stripe compaction - findbugs and javadoc issues, some improvements git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1540215 13f79535-47bb-0310-9956-ffa450edef68 --- .../hbase/regionserver/StoreFileManager.java | 2 +- .../hbase/regionserver/StripeStoreConfig.java | 12 ++++--- .../regionserver/StripeStoreFlusher.java | 4 ++- .../compactions/CompactionConfiguration.java | 6 ++-- .../compactions/StripeCompactionPolicy.java | 31 +------------------ .../TestStripeCompactionPolicy.java | 2 +- 6 files changed, 18 insertions(+), 39 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java index 85be8ae80ad..e2495b9b5cd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java @@ -48,7 +48,7 @@ public interface StoreFileManager { /** * Adds new files, either for from MemStore flush or bulk insert, into the structure. - * @param sf New store file. + * @param sfs New store files. */ void insertNewFiles(Collection sfs) throws IOException; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreConfig.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreConfig.java index e74a065ce9d..607344dc7d4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreConfig.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreConfig.java @@ -22,6 +22,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.regionserver.compactions.CompactionConfiguration; /** * Configuration class for stripe store and compactions. @@ -79,14 +80,17 @@ public class StripeStoreConfig { private static final double EPSILON = 0.001; // good enough for this, not a real epsilon. public StripeStoreConfig(Configuration config, StoreConfigInformation sci) { this.level0CompactMinFiles = config.getInt(MIN_FILES_L0_KEY, 4); - this.stripeCompactMinFiles = config.getInt(MIN_FILES_KEY, 3); - this.stripeCompactMaxFiles = config.getInt(MAX_FILES_KEY, 10); - this.maxRegionSplitImbalance = getFloat(config, MAX_REGION_SPLIT_IMBALANCE_KEY, 1.5f, true); this.flushIntoL0 = config.getBoolean(FLUSH_TO_L0_KEY, false); + int minMinFiles = flushIntoL0 ? 3 : 4; // make sure not to compact tiny files too often. + int minFiles = config.getInt(CompactionConfiguration.MIN_KEY, -1); + this.stripeCompactMinFiles = config.getInt(MIN_FILES_KEY, Math.max(minMinFiles, minFiles)); + this.stripeCompactMaxFiles = config.getInt(MAX_FILES_KEY, + config.getInt(CompactionConfiguration.MAX_KEY, 10)); + this.maxRegionSplitImbalance = getFloat(config, MAX_REGION_SPLIT_IMBALANCE_KEY, 1.5f, true); float splitPartCount = getFloat(config, SPLIT_PARTS_KEY, 2f, true); if (Math.abs(splitPartCount - 1.0) < EPSILON) { - LOG.error("Split part count cannot be 1 (" + this.splitPartCount + "), using the default"); + LOG.error("Split part count cannot be 1 (" + splitPartCount + "), using the default"); splitPartCount = 2f; } this.splitPartCount = splitPartCount; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java index cbfd07afc15..a2ece5d98bc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFlusher.java @@ -89,7 +89,9 @@ public class StripeStoreFlusher extends StoreFlusher { } } finally { if (!success && (mw != null)) { - result.clear(); + if (result != null) { + result.clear(); + } for (Path leftoverFile : mw.abortWriters()) { try { store.getFileSystem().delete(leftoverFile, false); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionConfiguration.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionConfiguration.java index 7b833ccdc12..69807121100 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionConfiguration.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionConfiguration.java @@ -48,6 +48,8 @@ public class CompactionConfiguration { private static final String CONFIG_PREFIX = "hbase.hstore.compaction."; public static final String RATIO_KEY = CONFIG_PREFIX + "ratio"; + public static final String MIN_KEY = CONFIG_PREFIX + "min"; + public static final String MAX_KEY = CONFIG_PREFIX + "max"; Configuration conf; StoreConfigInformation storeConfigInfo; @@ -70,9 +72,9 @@ public class CompactionConfiguration { maxCompactSize = conf.getLong(CONFIG_PREFIX + "max.size", Long.MAX_VALUE); minCompactSize = conf.getLong(CONFIG_PREFIX + "min.size", storeConfigInfo.getMemstoreFlushSize()); - minFilesToCompact = Math.max(2, conf.getInt(CONFIG_PREFIX + "min", + minFilesToCompact = Math.max(2, conf.getInt(MIN_KEY, /*old name*/ conf.getInt("hbase.hstore.compactionThreshold", 3))); - maxFilesToCompact = conf.getInt(CONFIG_PREFIX + "max", 10); + maxFilesToCompact = conf.getInt(MAX_KEY, 10); compactionRatio = conf.getFloat(RATIO_KEY, 1.2F); offPeekCompactionRatio = conf.getFloat(CONFIG_PREFIX + "ratio.offpeak", 5.0F); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java index 9b845306cfc..7189d216479 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java @@ -114,7 +114,7 @@ public class StripeCompactionPolicy extends CompactionPolicy { Collection allFiles = si.getStorefiles(); if (StoreUtils.hasReferences(allFiles)) { LOG.debug("There are references in the store; compacting all files"); - long targetKvs = estimateTargetKvs(allFiles, config.getSplitCount()).getFirst(); + long targetKvs = estimateTargetKvs(allFiles, config.getInitialCount()).getFirst(); SplitStripeCompactionRequest request = new SplitStripeCompactionRequest( allFiles, OPEN_KEY, OPEN_KEY, targetKvs); request.setMajorRangeFull(); @@ -509,35 +509,6 @@ public class StripeCompactionPolicy extends CompactionPolicy { } } - /** Helper class used to calculate size related things */ - private static class StripeSizes { - public final ArrayList kvCounts; - public final ArrayList fileSizes; - public double avgKvCount = 0; - public long minKvCount = Long.MAX_VALUE, maxKvCount = Long.MIN_VALUE; - public int minIndex = -1, maxIndex = -1; - - public StripeSizes(List> stripes) { - assert !stripes.isEmpty(); - kvCounts = new ArrayList(stripes.size()); - fileSizes = new ArrayList(stripes.size()); - for (int i = 0; i < stripes.size(); ++i) { - long kvCount = getTotalKvCount(stripes.get(i)); - fileSizes.add(getTotalFileSize(stripes.get(i))); - kvCounts.add(kvCount); - avgKvCount += (double)(kvCount - avgKvCount) / (i + 1); - if (minKvCount > kvCount) { - minIndex = i; - minKvCount = kvCount; - } - if (maxKvCount < kvCount) { - maxIndex = i; - maxKvCount = kvCount; - } - } - } - } - /** The information about stripes that the policy needs to do its stuff */ public static interface StripeInformationProvider { public Collection getStorefiles(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java index e3c1f97fdec..885d4503142 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java @@ -329,7 +329,7 @@ public class TestStripeCompactionPolicy { Configuration conf = HBaseConfiguration.create(); StripeCompactionPolicy policy = createPolicy(conf); // Verify the deletes can be dropped if there are no L0 files. - Long[][] stripes = new Long[][] { new Long[] { 3L, 2L, 2L }, new Long[] { 6L } }; + Long[][] stripes = new Long[][] { new Long[] { 3L, 2L, 2L, 2L }, new Long[] { 6L } }; StripeInformationProvider si = createStripesWithSizes(0, 0, stripes); verifySingleStripeCompaction(policy, si, 0, true); // But cannot be dropped if there are.