From 80273bd8e7b347bc9dafc30ce6a556a2def897f8 Mon Sep 17 00:00:00 2001 From: sershe Date: Mon, 4 Mar 2013 19:01:43 +0000 Subject: [PATCH] HBASE-7964 requestCompaction priority argument is not used (except for user compaction check) git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1452444 13f79535-47bb-0310-9956-ffa450edef68 --- .../hbase/regionserver/DefaultStoreEngine.java | 2 +- .../regionserver/DefaultStoreFileManager.java | 13 ++++++++++++- .../hadoop/hbase/regionserver/HStore.java | 17 +++++------------ .../apache/hadoop/hbase/regionserver/Store.java | 7 ------- .../hbase/regionserver/StoreFileManager.java | 5 +++++ .../compactions/CompactionConfiguration.java | 10 ---------- .../compactions/CompactionPolicy.java | 10 ---------- .../compactions/DefaultCompactionPolicy.java | 6 ------ 8 files changed, 23 insertions(+), 47 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java index 21906e6c04c..f2c5cd57c56 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreEngine.java @@ -46,7 +46,7 @@ public class DefaultStoreEngine extends StoreEngine< @Override protected void createComponents() { - storeFileManager = new DefaultStoreFileManager(this.comparator); + storeFileManager = new DefaultStoreFileManager(this.comparator, this.conf); // TODO: compactor and policy may be separately pluggable, but must derive from default ones. compactor = new DefaultCompactor(this.conf, this.store); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java index aad2bacbbe1..080faa46173 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java @@ -28,6 +28,7 @@ import java.util.List; 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.KeyValue; import org.apache.hadoop.hbase.KeyValue.KVComparator; @@ -43,6 +44,7 @@ class DefaultStoreFileManager implements StoreFileManager { static final Log LOG = LogFactory.getLog(DefaultStoreFileManager.class); private final KVComparator kvComparator; + private final Configuration conf; /** * List of store files inside this store. This is an immutable list that @@ -50,8 +52,9 @@ class DefaultStoreFileManager implements StoreFileManager { */ private volatile ImmutableList storefiles = null; - public DefaultStoreFileManager(KVComparator kvComparator) { + public DefaultStoreFileManager(KVComparator kvComparator, Configuration conf) { this.kvComparator = kvComparator; + this.conf = conf; } @Override @@ -124,9 +127,17 @@ class DefaultStoreFileManager implements StoreFileManager { return getStorefiles(); } + @Override + public int getStoreCompactionPriority() { + int blockingFileCount = conf.getInt( + "hbase.hstore.blockingStoreFiles", HStore.DEFAULT_BLOCKING_STOREFILE_COUNT); + return blockingFileCount - storefiles.size(); + } + private void sortAndSetStoreFiles(List storeFiles) { Collections.sort(storeFiles, StoreFile.Comparators.SEQ_ID); storefiles = ImmutableList.copyOf(storeFiles); } + } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index c968b8e0f31..c2f1a138913 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -1324,7 +1324,9 @@ public class HStore implements Store { this.forceMajor = this.forceMajor && !isMajor; // Set common request properties. - compaction.getRequest().setPriority(getCompactPriority(priority)); + // Set priority, either override value supplied by caller or from store. + compaction.getRequest().setPriority( + (priority != Store.NO_PRIORITY) ? priority : getCompactPriority()); compaction.getRequest().setIsMajor(isMajor); compaction.getRequest().setDescription( region.getRegionNameAsString(), getColumnFamilyName()); @@ -1755,18 +1757,9 @@ public class HStore implements Store { return this.memstore.heapSize(); } - public int getCompactPriority() { - return getCompactPriority(Store.NO_PRIORITY); - } - @Override - public int getCompactPriority(int priority) { - // If this is a user-requested compaction, leave this at the user priority - if (priority != Store.PRIORITY_USER) { - priority = this.compactionPolicy.getSystemCompactionPriority( - this.storeFileManager.getStorefiles()); - } - return priority; + public int getCompactPriority() { + return this.storeFileManager.getStoreCompactionPriority(); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java index ab16d873376..60d98bc3a9f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java @@ -182,13 +182,6 @@ public interface Store extends HeapSize, StoreConfigInformation { public int getCompactPriority(); - /** - * @param priority priority to check against. When priority is {@link Store#PRIORITY_USER}, - * {@link Store#PRIORITY_USER} is returned. - * @return The priority that this store has in the compaction queue. - */ - public int getCompactPriority(int priority); - public StoreFlusher getStoreFlusher(long cacheFlushId); // Split oriented methods 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 9fec93423d4..85faabaeaa5 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 @@ -119,4 +119,9 @@ public interface StoreFileManager { * @throws IOException */ public abstract byte[] getSplitPoint() throws IOException; + + /** + * @return The store compaction priority. + */ + public abstract int getStoreCompactionPriority(); } 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 d8d3c94f7f3..81cc3c091f3 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 @@ -62,7 +62,6 @@ public class CompactionConfiguration { boolean shouldDeleteExpired; long majorCompactionPeriod; float majorCompactionJitter; - int blockingStoreFileCount; CompactionConfiguration(Configuration conf, StoreConfigInformation storeConfigInfo) { this.conf = conf; @@ -82,8 +81,6 @@ public class CompactionConfiguration { shouldDeleteExpired = conf.getBoolean("hbase.store.delete.expired.storefile", true); majorCompactionPeriod = conf.getLong(HConstants.MAJOR_COMPACTION_PERIOD, 1000*60*60*24); majorCompactionJitter = conf.getFloat("hbase.hregion.majorcompaction.jitter", 0.20F); - blockingStoreFileCount = - conf.getInt("hbase.hstore.blockingStoreFiles", HStore.DEFAULT_BLOCKING_STOREFILE_COUNT); LOG.info("Compaction configuration " + this.toString()); } @@ -105,13 +102,6 @@ public class CompactionConfiguration { majorCompactionJitter); } - /** - * @return store file count that will cause the memstore of this store to be blocked. - */ - int getBlockingStorefileCount() { - return this.blockingStoreFileCount; - } - /** * @return lower bound below which compaction is selected without ratio test */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java index 7f98741876c..018e497a617 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionPolicy.java @@ -42,16 +42,6 @@ public abstract class CompactionPolicy { this.comConf = new CompactionConfiguration(conf, this.storeConfigInfo); } - /** - * @param storeFiles Store files in the store. - * @return The system compaction priority of the store, based on storeFiles. - * The priority range is as such - the smaller values are higher priority; - * 1 is user priority; only very important, blocking compactions should use - * values lower than that. With default settings, depending on the number of - * store files, the non-blocking priority will be in 2-6 range. - */ - public abstract int getSystemCompactionPriority(final Collection storeFiles); - /** * @param filesToCompact Files to compact. Can be null. * @return True if we should run a major compaction. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java index 898df3fa3ca..d92fa67326a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/DefaultCompactionPolicy.java @@ -57,7 +57,6 @@ public class DefaultCompactionPolicy extends CompactionPolicy { super(conf, storeConfigInfo); } - private ArrayList getCurrentEligibleFiles( ArrayList candidateFiles, final List filesCompacting) { // candidates = all storefiles not already in compaction queue @@ -77,11 +76,6 @@ public class DefaultCompactionPolicy extends CompactionPolicy { return getCurrentEligibleFiles(new ArrayList(candidates), filesCompacting); } - @Override - public int getSystemCompactionPriority(final Collection storeFiles) { - return this.comConf.getBlockingStorefileCount() - storeFiles.size(); - } - /** * @param candidateFiles candidate files, ordered from oldest to newest * @return subset copy of candidate list that meets compaction criteria