From c21c8a7b3c0ea4298e30b4ecad6f1aa743211aec Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Mon, 1 Jun 2020 15:27:00 -0700 Subject: [PATCH] PR Feedback: Extract NormalizeContext --- .../normalizer/SimpleRegionNormalizer.java | 122 +++++++++++------- 1 file changed, 75 insertions(+), 47 deletions(-) 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 7c70f2966d5d..525c404517c0 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 @@ -230,23 +230,20 @@ public List computePlansForTable(final TableName table) { return Collections.emptyList(); } - final List plans = new ArrayList<>(); - final List 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 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 computeMergeNormalizationPlans(final TableName table, - final List tableRegions) { - final RegionStates regionStates = masterServices.getAssignmentManager().getRegionStates(); - - if (tableRegions.size() < minRegionCount) { + private List 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 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. + *
+ * if the region is > 2 times larger than average, we split it. split + * is more high priority normalization action than merge. */ - private List computeSplitNormalizationPlans(final TableName table, - final List tableRegions) { - final RegionStates regionStates = masterServices.getAssignmentManager().getRegionStates(); - final double avgRegionSize = getAverageRegionSizeMb(tableRegions); - LOG.debug("Table {}, average region size: {}", table, avgRegionSize); + private List computeSplitNormalizationPlans(final NormalizeContext ctx) { + final double avgRegionSize = ctx.getAverageRegionSizeMb(); + LOG.debug("Table {}, average region size: {}", ctx.getTableName(), avgRegionSize); final List 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 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 getTableRegions() { + return tableRegions; + } + + public double getAverageRegionSizeMb() { + return averageRegionSizeMb; + } + } }