HBASE-25745 Deprecate/Rename config hbase.normalizer.min.region.count to hbase.normalizer.merge.min.region.count

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

hbase-common/src/main/java/org/apache/hadoop/hbase/HBaseConfiguration.java

@@ -113,7 +113,9 @@ public class HBaseConfiguration extends Configuration {
         "hbase.server.allocator.buffer.size"),
       new DeprecationDelta("hlog.bulk.output", "wal.bulk.output"),
       new DeprecationDelta("hlog.input.tables", "wal.input.tables"),
-      new DeprecationDelta("hlog.input.tablesmap", "wal.input.tablesmap")
+      new DeprecationDelta("hlog.input.tablesmap", "wal.input.tablesmap"),
+      new DeprecationDelta("hbase.normalizer.min.region.count",
+        "hbase.normalizer.merge.min.region.count")
     });
   }
 

hbase-common/src/main/resources/hbase-default.xml

@@ -640,7 +640,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>
+    <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>

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 <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";
@@ -101,11 +106,12 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
     setConf(conf);
   }
 
-  private static int parseMinRegionCount(final Configuration conf) {
+  private static int parseMergeMinRegionCount(final Configuration conf) {
-    final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+    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 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
   }
 
   /**
-   * 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() {
+  public int getMergeMinRegionCount() {
-    return normalizerConfiguration.getMinRegionCount();
+    return normalizerConfiguration.getMergeMinRegionCount();
   }
 
   /**
@@ -340,10 +346,10 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
    */
   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();
     }
 
@@ -493,7 +499,7 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
     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 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
       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 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
       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(),
+      logConfigurationUpdated(MERGE_MIN_REGION_COUNT_KEY,
-        minRegionCount);
+        currentConfiguration.getMergeMinRegionCount(), mergeMinRegionCount);
       logConfigurationUpdated(MERGE_MIN_REGION_AGE_DAYS_KEY,
         currentConfiguration.getMergeMinRegionAge(), mergeMinRegionAge);
       logConfigurationUpdated(MERGE_MIN_REGION_SIZE_MB_KEY,
@@ -540,16 +546,26 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
       return mergeEnabled;
     }
 
-    public int getMinRegionCount() {
+    public int getMergeMinRegionCount() {
-      return minRegionCount;
+      return mergeMinRegionCount;
     }
 
-    public int getMinRegionCount(NormalizeContext context) {
+    public int getMergeMinRegionCount(NormalizeContext context) {
-      int minRegionCount = context.getOrDefault(MIN_REGION_COUNT_KEY, Integer::parseInt, 0);
+      String stringValue = context.getOrDefault(MERGE_MIN_REGION_COUNT_KEY,
-      if (minRegionCount <= 0) {
+        Function.identity(), null);
-        minRegionCount = getMinRegionCount();
+      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);
+        }
       }
-      return minRegionCount;
+      final int mergeMinRegionCount = stringValue == null ? 0 : Integer.parseInt(stringValue);
+      if (mergeMinRegionCount <= 0) {
+        return getMergeMinRegionCount();
+      }
+      return mergeMinRegionCount;
     }
 
     public Period getMergeMinRegionAge() {
@@ -557,7 +573,7 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
     }
 
     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 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
     }
 
     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;
     }

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}.
  *       </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},
+ *         Configuration: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#MERGE_MIN_REGION_COUNT_KEY},
- *         default: {@value org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer#DEFAULT_MIN_REGION_COUNT}.
+ *         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},

hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerManagerConfigurationObserver.java

@@ -77,19 +77,19 @@ public class TestRegionNormalizerManagerConfigurationObserver {
   @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()));
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java

@@ -21,6 +21,7 @@ import static java.lang.String.format;
 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 @@ public class TestSimpleRegionNormalizer {
     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());
@@ -370,7 +371,19 @@ public class TestSimpleRegionNormalizer {
 
   @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
@@ -378,6 +391,7 @@ public class TestSimpleRegionNormalizer {
     // 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(
@@ -388,15 +402,29 @@ public class TestSimpleRegionNormalizer {
         .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
@@ -404,6 +432,7 @@ public class TestSimpleRegionNormalizer {
     // 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(
@@ -413,7 +442,7 @@ public class TestSimpleRegionNormalizer {
         .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)));
   }