PR Feedback: Extract NormalizeContext

apache · Jun 1, 2020 · c21c8a7 · c21c8a7
1 parent fde0a68
commit c21c8a7
Showing 1 changed file with 75 additions and 47 deletions.
diff --git a/...erver/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java b/...erver/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
@@ -230,23 +230,20 @@ public List<NormalizationPlan> computePlansForTable(final TableName table) {
       return Collections.emptyList();
     }
 
-    final List<NormalizationPlan> plans = new ArrayList<>();
-    final List<RegionInfo> tableRegions = masterServices.getAssignmentManager()
-      .getRegionStates()
-      .getRegionsOfTable(table);
-
-    if (CollectionUtils.isEmpty(tableRegions)) {
+    final NormalizeContext ctx = new NormalizeContext(table);
+    if (CollectionUtils.isEmpty(ctx.getTableRegions())) {
       return Collections.emptyList();
     }
 
     LOG.debug("Computing normalization plan for table:  {}, number of regions: {}", table,
-      tableRegions.size());
+      ctx.getTableRegions().size());
 
+    final List<NormalizationPlan> plans = new ArrayList<>();
     if (proceedWithSplitPlanning) {
-      plans.addAll(computeSplitNormalizationPlans(table, tableRegions));
+      plans.addAll(computeSplitNormalizationPlans(ctx));
     }
     if (proceedWithMergePlanning) {
-      plans.addAll(computeMergeNormalizationPlans(table, tableRegions));
+      plans.addAll(computeMergeNormalizationPlans(ctx));
     }
 
     LOG.debug("Computed {} normalization plans for table {}", plans.size(), table);
@@ -348,36 +345,26 @@ private boolean skipForMerge(final RegionStates regionStates, final RegionInfo r
 
   /**
    * Computes the merge plans that should be executed for this table to converge average region
-   * towards target average or target region count
-   * @param table table to normalize
-   * @return list of merge normalization plans
+   * towards target average or target region count.
    */
-  private List<NormalizationPlan> computeMergeNormalizationPlans(final TableName table,
-    final List<RegionInfo> tableRegions) {
-    final RegionStates regionStates = masterServices.getAssignmentManager().getRegionStates();
-
-    if (tableRegions.size() < minRegionCount) {
+  private List<NormalizationPlan> computeMergeNormalizationPlans(final NormalizeContext ctx) {
+    if (ctx.getTableRegions().size() < minRegionCount) {
       LOG.debug("Table {} has {} regions, required min number of regions for normalizer to run"
-        + " is {}, not computing merge plans.", table, tableRegions.size(), minRegionCount);
+        + " is {}, not computing merge plans.", ctx.getTableName(), ctx.getTableRegions().size(),
+        minRegionCount);
       return Collections.emptyList();
     }
 
-    final double avgRegionSizeMb = getAverageRegionSizeMb(tableRegions);
-    LOG.debug("Table {}, average region size: {}. Computing normalization plan for table: {}, "
-        + "number of regions: {}.",
-      table, avgRegionSizeMb, table, tableRegions.size());
-
-    // The list of regionInfo from getRegionsOfTable() is ordered by regionName.
-    // regionName does not necessary guarantee the order by STARTKEY (let's say 'aa1', 'aa1!',
-    // in order by regionName, it will be 'aa1!' followed by 'aa1').
-    // This could result in normalizer merging non-adjacent regions into one and creates overlaps.
-    // In order to avoid that, sort the list by RegionInfo.COMPARATOR.
-    tableRegions.sort(RegionInfo.COMPARATOR);
+    final double avgRegionSizeMb = ctx.getAverageRegionSizeMb();
+    LOG.debug("Computing normalization plan for table {}. average region size: {}, number of"
+      + " regions: {}.", ctx.getTableName(), avgRegionSizeMb, ctx.getTableRegions().size());
+
     final List<NormalizationPlan> plans = new ArrayList<>();
-    for (int candidateIdx = 0; candidateIdx < tableRegions.size() - 1; candidateIdx++) {
-      final RegionInfo current = tableRegions.get(candidateIdx);
-      final RegionInfo next = tableRegions.get(candidateIdx + 1);
-      if (skipForMerge(regionStates, current) || skipForMerge(regionStates, next)) {
+    for (int candidateIdx = 0; candidateIdx < ctx.getTableRegions().size() - 1; candidateIdx++) {
+      final RegionInfo current = ctx.getTableRegions().get(candidateIdx);
+      final RegionInfo next = ctx.getTableRegions().get(candidateIdx + 1);
+      if (skipForMerge(ctx.getRegionStates(), current)
+        || skipForMerge(ctx.getRegionStates(), next)) {
         continue;
       }
       final long currentSizeMb = getRegionSizeMB(current);
@@ -406,27 +393,24 @@ private static boolean skipForSplit(final RegionState state, final RegionInfo re
 
   /**
    * Computes the split plans that should be executed for this table to converge average region size
-   * towards target average or target region count
-   * @param table table to normalize
-   * @return list of split normalization plans
+   * towards target average or target region count.
+   * <br />
+   * if the region is > 2 times larger than average, we split it. split
+   * is more high priority normalization action than merge.
    */
-  private List<NormalizationPlan> computeSplitNormalizationPlans(final TableName table,
-    final List<RegionInfo> tableRegions) {
-    final RegionStates regionStates = masterServices.getAssignmentManager().getRegionStates();
-    final double avgRegionSize = getAverageRegionSizeMb(tableRegions);
-    LOG.debug("Table {}, average region size: {}", table, avgRegionSize);
+  private List<NormalizationPlan> computeSplitNormalizationPlans(final NormalizeContext ctx) {
+    final double avgRegionSize = ctx.getAverageRegionSizeMb();
+    LOG.debug("Table {}, average region size: {}", ctx.getTableName(), avgRegionSize);
 
     final List<NormalizationPlan> plans = new ArrayList<>();
-    for (final RegionInfo hri : tableRegions) {
-      if (skipForSplit(regionStates.getRegionState(hri), hri)) {
+    for (final RegionInfo hri : ctx.getTableRegions()) {
+      if (skipForSplit(ctx.getRegionStates().getRegionState(hri), hri)) {
         continue;
       }
-      long regionSize = getRegionSizeMB(hri);
-      // if the region is > 2 times larger than average, we split it, split
-      // is more high priority normalization action than merge.
+      final long regionSize = getRegionSizeMB(hri);
       if (regionSize > 2 * avgRegionSize) {
         LOG.info("Table {}, large region {} has size {}, more than twice avg size {}, splitting",
-          table, hri.getRegionNameAsString(), regionSize, avgRegionSize);
+          ctx.getTableName(), hri.getRegionNameAsString(), regionSize, avgRegionSize);
         plans.add(new SplitNormalizationPlan(hri, null));
       }
     }
@@ -459,4 +443,48 @@ private static boolean logTraceReason(final BooleanSupplier predicate, final Str
     }
     return value;
   }
+
+  /**
+   * Inner class caries the state necessary to perform a single invocation of
+   * {@link #computePlansForTable(TableName)}. Grabbing this data from the assignment manager
+   * up-front allows any computed values to be realized just once.
+   */
+  private class NormalizeContext {
+    private final TableName tableName;
+    private final RegionStates regionStates;
+    private final List<RegionInfo> tableRegions;
+    private final double averageRegionSizeMb;
+
+    public NormalizeContext(final TableName tableName) {
+      this.tableName = tableName;
+      regionStates = SimpleRegionNormalizer.this.masterServices
+        .getAssignmentManager()
+        .getRegionStates();
+      tableRegions = regionStates.getRegionsOfTable(tableName);
+      // The list of regionInfo from getRegionsOfTable() is ordered by regionName.
+      // regionName does not necessary guarantee the order by STARTKEY (let's say 'aa1', 'aa1!',
+      // in order by regionName, it will be 'aa1!' followed by 'aa1').
+      // This could result in normalizer merging non-adjacent regions into one and creates overlaps.
+      // In order to avoid that, sort the list by RegionInfo.COMPARATOR.
+      // See HBASE-24376
+      tableRegions.sort(RegionInfo.COMPARATOR);
+      averageRegionSizeMb = SimpleRegionNormalizer.this.getAverageRegionSizeMb(this.tableRegions);
+    }
+
+    public TableName getTableName() {
+      return tableName;
+    }
+
+    public RegionStates getRegionStates() {
+      return regionStates;
+    }
+
+    public List<RegionInfo> getTableRegions() {
+      return tableRegions;
+    }
+
+    public double getAverageRegionSizeMb() {
+      return averageRegionSizeMb;
+    }
+  }
 }