From c60a81a0707d64c2c7a8f5799f1461abcb450601 Mon Sep 17 00:00:00 2001 From: Baiqiang Zhao Date: Fri, 9 Apr 2021 15:15:53 +0800 Subject: [PATCH] HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.count` to `hbase.normalizer.merge.min.region.count` --- .../hadoop/hbase/HBaseConfiguration.java | 4 +- .../src/main/resources/hbase-default.xml | 2 +- .../normalizer/SimpleRegionNormalizer.java | 68 ++++++++++++------- .../hbase/master/normalizer/package-info.java | 4 +- ...ormalizerManagerConfigurationObserver.java | 6 +- .../TestSimpleRegionNormalizer.java | 39 +++++++++-- 6 files changed, 85 insertions(+), 38 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java index 1ca7ce6dbc4b..cb111d29cd6f 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java @@ -80,7 +80,9 @@ private static void addDeprecatedKeys() { new DeprecationDelta("hlog.input.tables", "wal.input.tables"), new DeprecationDelta("hlog.input.tablesmap", "wal.input.tablesmap"), new DeprecationDelta("hbase.master.mob.ttl.cleaner.period", - "hbase.master.mob.cleaner.period") + "hbase.master.mob.cleaner.period"), + new DeprecationDelta("hbase.normalizer.min.region.count", + "hbase.normalizer.merge.min.region.count") }); } diff --git a/hbase-common/src/main/resources/hbase-default.xml b/hbase-common/src/main/resources/hbase-default.xml index 6dd644052662..176d9ba72cda 100644 --- a/hbase-common/src/main/resources/hbase-default.xml +++ b/hbase-common/src/main/resources/hbase-default.xml @@ -639,7 +639,7 @@ possible configurations would overwhelm and obscure the important. Whether to merge a region as part of normalization. - hbase.normalizer.min.region.count + hbase.normalizer.merge.min.region.count 3 The minimum number of regions in a table to consider it for merge normalization. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java index 55c1cd5657f6..d6bd00440636 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java @@ -65,10 +65,15 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver static final boolean DEFAULT_SPLIT_ENABLED = true; static final String MERGE_ENABLED_KEY = "hbase.normalizer.merge.enabled"; static final boolean DEFAULT_MERGE_ENABLED = true; - // TODO: after HBASE-24416, `min.region.count` only applies to merge plans; should - // deprecate/rename the configuration key. + /** + * @deprecated since 2.5.0 and will be removed in 4.0.0. + * Use {@link SimpleRegionNormalizer#MERGE_MIN_REGION_COUNT_KEY} instead. + * @see HBASE-25745 + */ + @Deprecated static final String MIN_REGION_COUNT_KEY = "hbase.normalizer.min.region.count"; - static final int DEFAULT_MIN_REGION_COUNT = 3; + static final String MERGE_MIN_REGION_COUNT_KEY = "hbase.normalizer.merge.min.region.count"; + static final int DEFAULT_MERGE_MIN_REGION_COUNT = 3; static final String MERGE_MIN_REGION_AGE_DAYS_KEY = "hbase.normalizer.merge.min_region_age.days"; static final int DEFAULT_MERGE_MIN_REGION_AGE_DAYS = 3; static final String MERGE_MIN_REGION_SIZE_MB_KEY = "hbase.normalizer.merge.min_region_size.mb"; @@ -101,11 +106,12 @@ public void onConfigurationChange(Configuration conf) { setConf(conf); } - private static int parseMinRegionCount(final Configuration conf) { - final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT); + private static int parseMergeMinRegionCount(final Configuration conf) { + final int parsedValue = conf.getInt(MERGE_MIN_REGION_COUNT_KEY, + DEFAULT_MERGE_MIN_REGION_COUNT); final int settledValue = Math.max(1, parsedValue); if (parsedValue != settledValue) { - warnInvalidValue(MIN_REGION_COUNT_KEY, parsedValue, settledValue); + warnInvalidValue(MERGE_MIN_REGION_COUNT_KEY, parsedValue, settledValue); } return settledValue; } @@ -158,10 +164,10 @@ public boolean isMergeEnabled() { } /** - * Return this instance's configured value for {@value #MIN_REGION_COUNT_KEY}. + * Return this instance's configured value for {@value #MERGE_MIN_REGION_COUNT_KEY}. */ - public int getMinRegionCount() { - return normalizerConfiguration.getMinRegionCount(); + public int getMergeMinRegionCount() { + return normalizerConfiguration.getMergeMinRegionCount(); } /** @@ -340,10 +346,10 @@ private boolean skipForMerge( */ private List computeMergeNormalizationPlans(final NormalizeContext ctx) { final NormalizerConfiguration configuration = normalizerConfiguration; - if (ctx.getTableRegions().size() < configuration.getMinRegionCount(ctx)) { + if (ctx.getTableRegions().size() < configuration.getMergeMinRegionCount(ctx)) { LOG.debug("Table {} has {} regions, required min number of regions for normalizer to run" + " is {}, not computing merge plans.", ctx.getTableName(), - ctx.getTableRegions().size(), configuration.getMinRegionCount()); + ctx.getTableRegions().size(), configuration.getMergeMinRegionCount()); return Collections.emptyList(); } @@ -493,7 +499,7 @@ private static final class NormalizerConfiguration { private final Configuration conf; private final boolean splitEnabled; private final boolean mergeEnabled; - private final int minRegionCount; + private final int mergeMinRegionCount; private final Period mergeMinRegionAge; private final long mergeMinRegionSizeMb; @@ -501,7 +507,7 @@ private NormalizerConfiguration() { conf = null; splitEnabled = DEFAULT_SPLIT_ENABLED; mergeEnabled = DEFAULT_MERGE_ENABLED; - minRegionCount = DEFAULT_MIN_REGION_COUNT; + mergeMinRegionCount = DEFAULT_MERGE_MIN_REGION_COUNT; mergeMinRegionAge = Period.ofDays(DEFAULT_MERGE_MIN_REGION_AGE_DAYS); mergeMinRegionSizeMb = DEFAULT_MERGE_MIN_REGION_SIZE_MB; } @@ -513,15 +519,15 @@ private NormalizerConfiguration( this.conf = conf; splitEnabled = conf.getBoolean(SPLIT_ENABLED_KEY, DEFAULT_SPLIT_ENABLED); mergeEnabled = conf.getBoolean(MERGE_ENABLED_KEY, DEFAULT_MERGE_ENABLED); - minRegionCount = parseMinRegionCount(conf); + mergeMinRegionCount = parseMergeMinRegionCount(conf); mergeMinRegionAge = parseMergeMinRegionAge(conf); mergeMinRegionSizeMb = parseMergeMinRegionSizeMb(conf); logConfigurationUpdated(SPLIT_ENABLED_KEY, currentConfiguration.isSplitEnabled(), splitEnabled); logConfigurationUpdated(MERGE_ENABLED_KEY, currentConfiguration.isMergeEnabled(), mergeEnabled); - logConfigurationUpdated(MIN_REGION_COUNT_KEY, currentConfiguration.getMinRegionCount(), - minRegionCount); + logConfigurationUpdated(MERGE_MIN_REGION_COUNT_KEY, + currentConfiguration.getMergeMinRegionCount(), mergeMinRegionCount); logConfigurationUpdated(MERGE_MIN_REGION_AGE_DAYS_KEY, currentConfiguration.getMergeMinRegionAge(), mergeMinRegionAge); logConfigurationUpdated(MERGE_MIN_REGION_SIZE_MB_KEY, @@ -540,16 +546,26 @@ public boolean isMergeEnabled() { return mergeEnabled; } - public int getMinRegionCount() { - return minRegionCount; + public int getMergeMinRegionCount() { + return mergeMinRegionCount; } - public int getMinRegionCount(NormalizeContext context) { - int minRegionCount = context.getOrDefault(MIN_REGION_COUNT_KEY, Integer::parseInt, 0); - if (minRegionCount <= 0) { - minRegionCount = getMinRegionCount(); + public int getMergeMinRegionCount(NormalizeContext context) { + String stringValue = context.getOrDefault(MERGE_MIN_REGION_COUNT_KEY, + Function.identity(), null); + if (stringValue == null) { + stringValue = context.getOrDefault(MIN_REGION_COUNT_KEY, Function.identity(), null); + if (stringValue != null) { + LOG.debug("The config key {} in table descriptor is deprecated. Instead please use {}. " + + "In future release we will remove the deprecated config.", MIN_REGION_COUNT_KEY, + MERGE_MIN_REGION_COUNT_KEY); + } + } + final int mergeMinRegionCount = stringValue == null ? 0 : Integer.parseInt(stringValue); + if (mergeMinRegionCount <= 0) { + return getMergeMinRegionCount(); } - return minRegionCount; + return mergeMinRegionCount; } public Period getMergeMinRegionAge() { @@ -557,7 +573,7 @@ public Period getMergeMinRegionAge() { } public Period getMergeMinRegionAge(NormalizeContext context) { - int mergeMinRegionAge = context.getOrDefault(MERGE_MIN_REGION_AGE_DAYS_KEY, + final int mergeMinRegionAge = context.getOrDefault(MERGE_MIN_REGION_AGE_DAYS_KEY, Integer::parseInt, -1); if (mergeMinRegionAge < 0) { return getMergeMinRegionAge(); @@ -570,10 +586,10 @@ public long getMergeMinRegionSizeMb() { } public long getMergeMinRegionSizeMb(NormalizeContext context) { - long mergeMinRegionSizeMb = context.getOrDefault(MERGE_MIN_REGION_SIZE_MB_KEY, + final long mergeMinRegionSizeMb = context.getOrDefault(MERGE_MIN_REGION_SIZE_MB_KEY, Long::parseLong, (long)-1); if (mergeMinRegionSizeMb < 0) { - mergeMinRegionSizeMb = getMergeMinRegionSizeMb(); + return getMergeMinRegionSizeMb(); } return mergeMinRegionSizeMb; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/package-info.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/package-info.java index 21741c9f57cf..81cb6f407c48 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/package-info.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/package-info.java @@ -47,8 +47,8 @@ * default: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#DEFAULT_MERGE_ENABLED}. * *
  • The minimum number of regions in a table to consider it for merge normalization. - * Configuration: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#MIN_REGION_COUNT_KEY}, - * default: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#DEFAULT_MIN_REGION_COUNT}. + * Configuration: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#MERGE_MIN_REGION_COUNT_KEY}, + * default: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#DEFAULT_MERGE_MIN_REGION_COUNT}. *
    • *
  • The minimum age for a region to be considered for a merge, in days. Configuration: * {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#MERGE_MIN_REGION_AGE_DAYS_KEY}, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java index 00980233edce..cfbd6bd42257 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java @@ -77,19 +77,19 @@ public void before() { @Test public void test() { assertTrue(normalizer.isMergeEnabled()); - assertEquals(3, normalizer.getMinRegionCount()); + assertEquals(3, normalizer.getMergeMinRegionCount()); assertEquals(1_000_000L, parseConfiguredRateLimit(worker.getRateLimiter())); final Configuration newConf = new Configuration(conf); // configs on SimpleRegionNormalizer newConf.setBoolean("hbase.normalizer.merge.enabled", false); - newConf.setInt("hbase.normalizer.min.region.count", 100); + newConf.setInt("hbase.normalizer.merge.min.region.count", 100); // config on RegionNormalizerWorker newConf.set("hbase.normalizer.throughput.max_bytes_per_sec", "12g"); configurationManager.notifyAllObservers(newConf); assertFalse(normalizer.isMergeEnabled()); - assertEquals(100, normalizer.getMinRegionCount()); + assertEquals(100, normalizer.getMergeMinRegionCount()); assertEquals(12_884L, parseConfiguredRateLimit(worker.getRateLimiter())); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java index 2db68342dd65..7cbfba7a1d5c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java @@ -21,6 +21,7 @@ import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.DEFAULT_MERGE_MIN_REGION_AGE_DAYS; import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.MERGE_ENABLED_KEY; import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.MERGE_MIN_REGION_AGE_DAYS_KEY; +import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.MERGE_MIN_REGION_COUNT_KEY; import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.MERGE_MIN_REGION_SIZE_MB_KEY; import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.MIN_REGION_COUNT_KEY; import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.SPLIT_ENABLED_KEY; @@ -137,7 +138,7 @@ private void noNormalizationOnTransitioningRegions(final RegionState.State state when(masterServices.getAssignmentManager().getRegionStates() .getRegionState(any(RegionInfo.class))) .thenReturn(RegionState.createForTesting(null, state)); - assertThat(normalizer.getMinRegionCount(), greaterThanOrEqualTo(regionInfos.size())); + assertThat(normalizer.getMergeMinRegionCount(), greaterThanOrEqualTo(regionInfos.size())); List plans = normalizer.computePlansForTable(tableDescriptor); assertThat(format("Unexpected plans for RegionState %s", state), plans, empty()); @@ -370,7 +371,19 @@ public void testHonorsMergeEnabledInTD() { @Test public void testHonorsMinimumRegionCount() { - conf.setInt(MIN_REGION_COUNT_KEY, 1); + honorsMinimumRegionCount(MERGE_MIN_REGION_COUNT_KEY); + } + + /** + * Test the backward compatibility of the deprecated MIN_REGION_COUNT_KEY configuration. + */ + @Test + public void testHonorsOldMinimumRegionCount() { + honorsMinimumRegionCount(MIN_REGION_COUNT_KEY); + } + + private void honorsMinimumRegionCount(String confKey) { + conf.setInt(confKey, 1); final TableName tableName = name.getTableName(); final List regionInfos = createRegionInfos(tableName, 3); // create a table topology that results in both a merge plan and a split plan. Assert that the @@ -378,6 +391,7 @@ public void testHonorsMinimumRegionCount() { // threshold, and that the split plan is create in both cases. final Map regionSizes = createRegionSizesMap(regionInfos, 1, 1, 10); setupMocksForNormalizer(regionSizes, regionInfos); + assertEquals(1, normalizer.getMergeMinRegionCount()); List plans = normalizer.computePlansForTable(tableDescriptor); assertThat(plans, contains( @@ -388,15 +402,29 @@ public void testHonorsMinimumRegionCount() { .build())); // have to call setupMocks again because we don't have dynamic config update on normalizer. - conf.setInt(MIN_REGION_COUNT_KEY, 4); + conf.setInt(confKey, 4); setupMocksForNormalizer(regionSizes, regionInfos); + assertEquals(4, normalizer.getMergeMinRegionCount()); assertThat(normalizer.computePlansForTable(tableDescriptor), contains( new SplitNormalizationPlan(regionInfos.get(2), 10))); } @Test public void testHonorsMinimumRegionCountInTD() { - conf.setInt(MIN_REGION_COUNT_KEY, 1); + honorsOldMinimumRegionCountInTD(MERGE_MIN_REGION_COUNT_KEY); + } + + /** + * Test the backward compatibility of the deprecated MIN_REGION_COUNT_KEY configuration in table + * descriptor. + */ + @Test + public void testHonorsOldMinimumRegionCountInTD() { + honorsOldMinimumRegionCountInTD(MIN_REGION_COUNT_KEY); + } + + private void honorsOldMinimumRegionCountInTD(String confKey) { + conf.setInt(confKey, 1); final TableName tableName = name.getTableName(); final List regionInfos = createRegionInfos(tableName, 3); // create a table topology that results in both a merge plan and a split plan. Assert that the @@ -404,6 +432,7 @@ public void testHonorsMinimumRegionCountInTD() { // threshold, and that the split plan is create in both cases. final Map regionSizes = createRegionSizesMap(regionInfos, 1, 1, 10); setupMocksForNormalizer(regionSizes, regionInfos); + assertEquals(1, normalizer.getMergeMinRegionCount()); List plans = normalizer.computePlansForTable(tableDescriptor); assertThat(plans, contains( @@ -413,7 +442,7 @@ public void testHonorsMinimumRegionCountInTD() { .addTarget(regionInfos.get(1), 1) .build())); - when(tableDescriptor.getValue(MIN_REGION_COUNT_KEY)).thenReturn("4"); + when(tableDescriptor.getValue(confKey)).thenReturn("4"); assertThat(normalizer.computePlansForTable(tableDescriptor), contains( new SplitNormalizationPlan(regionInfos.get(2), 10))); }