Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun… #3139

Merged
merged 1 commit into from
May 24, 2021
Merged

HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun… #3139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
});
}

Expand Down
2 changes: 1 addition & 1 deletion hbase-common/src/main/resources/hbase-default.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ possible configurations would overwhelm and obscure the important.
<description>Whether to merge a region as part of normalization.</description>
</property>
<property>
<name>hbase.normalizer.min.region.count</name>
ndimiduk marked this conversation as resolved.
Show resolved Hide resolved
<name>hbase.normalizer.merge.min.region.count</name>
<value>3</value>
<description>The minimum number of regions in a table to consider it for merge
normalization.</description>
Expand Down
68 changes: 42 additions & 26 deletions ...erver/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href="https://issues.apache.org/jira/browse/HBASE-25745">HBASE-25745</a>
*/
@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";
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -340,10 +346,10 @@ private boolean skipForMerge(
*/
private List<NormalizationPlan> 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();
}

Expand Down Expand Up @@ -493,15 +499,15 @@ 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;

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;
}
Expand All @@ -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,
Expand All @@ -540,24 +546,34 @@ 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 {}. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh. maybe TableDescriptor should handle deprecations as well. For another patch.

+ "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() {
return mergeMinRegionAge;
}

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();
Expand All @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/package-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
* default: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#DEFAULT_MERGE_ENABLED}.
* </li>
* <li>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}.
* </li>
* <li>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},
Expand Down
6 changes: 3 additions & 3 deletions ...ache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}

Expand Down
39 changes: 34 additions & 5 deletions ...r/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<NormalizationPlan> plans = normalizer.computePlansForTable(tableDescriptor);
assertThat(format("Unexpected plans for RegionState %s", state), plans, empty());
Expand Down Expand Up @@ -370,14 +371,27 @@ 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<RegionInfo> regionInfos = createRegionInfos(tableName, 3);
// create a table topology that results in both a merge plan and a split plan. Assert that the
// merge is only created when the when the number of table regions is above the region count
// threshold, and that the split plan is create in both cases.
final Map<byte[], Integer> regionSizes = createRegionSizesMap(regionInfos, 1, 1, 10);
setupMocksForNormalizer(regionSizes, regionInfos);
assertEquals(1, normalizer.getMergeMinRegionCount());

List<NormalizationPlan> plans = normalizer.computePlansForTable(tableDescriptor);
assertThat(plans, contains(
Expand All @@ -388,22 +402,37 @@ 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<RegionInfo> regionInfos = createRegionInfos(tableName, 3);
// create a table topology that results in both a merge plan and a split plan. Assert that the
// merge is only created when the when the number of table regions is above the region count
// threshold, and that the split plan is create in both cases.
final Map<byte[], Integer> regionSizes = createRegionSizesMap(regionInfos, 1, 1, 10);
setupMocksForNormalizer(regionSizes, regionInfos);
assertEquals(1, normalizer.getMergeMinRegionCount());

List<NormalizationPlan> plans = normalizer.computePlansForTable(tableDescriptor);
assertThat(plans, contains(
Expand All @@ -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)));
}
Expand Down