From b169ee8a3e5497eeeb0bfc6af5167a6e84789f1b Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Tue, 15 Nov 2022 21:14:07 -0500 Subject: [PATCH 1/6] Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer This commit adds two new settings: - hbase.normalizer.merge.plans_size_limit.mb - hbase.normalizer.split.plans_size_limit.mb These settings limit the number of plans produced by a single run of SimpleRegionNormalizer, by forcing it to stop producing new plans once the cumulative region size limits are exceeded. --- .../normalizer/SimpleRegionNormalizer.java | 45 ++++++++++++++++++- .../TestSimpleRegionNormalizer.java | 37 +++++++++++++++ 2 files changed, 80 insertions(+), 2 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 c55f9ebdaca2..388c4adac621 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 @@ -22,6 +22,7 @@ import java.time.Instant; import java.time.Period; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -79,6 +80,10 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver 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"; static final int DEFAULT_MERGE_MIN_REGION_SIZE_MB = 0; + static final String CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.merge.plans_size_limit.mb"; + static final long DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB = Long.MAX_VALUE; + static final String CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY = "hbase.normalizer.split.plans_size_limit.mb"; + static final long DEFAULT_CUMULATIVE_SPLIT_SIZE_LIMIT_MB = Long.MAX_VALUE; private MasterServices masterServices; private NormalizerConfiguration normalizerConfiguration; @@ -234,6 +239,14 @@ public List computePlansForTable(final TableDescriptor tableD return plans; } + private long getTotalRegionSizeMB(Collection targets) { + long total = 0; + for (NormalizationTarget target : targets) { + total += target.getRegionSizeMb(); + } + return total; + } + /** Returns size of region in MB and if region is not found than -1 */ private long getRegionSizeMB(RegionInfo hri) { ServerName sn = @@ -357,6 +370,7 @@ private List computeMergeNormalizationPlans(final NormalizeCo final List plans = new LinkedList<>(); final List rangeMembers = new LinkedList<>(); long sumRangeMembersSizeMb; + long cumulativePlansSizeMb = 0; int current = 0; for (int rangeStart = 0; rangeStart < ctx.getTableRegions().size() - 1 && current < ctx.getTableRegions().size();) { @@ -394,7 +408,12 @@ private List computeMergeNormalizationPlans(final NormalizeCo break; } if (rangeMembers.size() > 1) { - plans.add(new MergeNormalizationPlan.Builder().setTargets(rangeMembers).build()); + cumulativePlansSizeMb += getTotalRegionSizeMB(rangeMembers); + if (cumulativePlansSizeMb > normalizerConfiguration.getCumulativeMergePlansSizeLimitMb()) { + return plans; + } else { + plans.add(new MergeNormalizationPlan.Builder().setTargets(rangeMembers).build()); + } } } return plans; @@ -422,6 +441,7 @@ private List computeSplitNormalizationPlans(final NormalizeCo LOG.debug("Table {}, average region size: {} MB", ctx.getTableName(), String.format("%.3f", avgRegionSize)); + long cumulativePlansSizeMb = 0; final List plans = new ArrayList<>(); for (final RegionInfo hri : ctx.getTableRegions()) { if (skipForSplit(ctx.getRegionStates().getRegionState(hri), hri)) { @@ -434,7 +454,12 @@ private List computeSplitNormalizationPlans(final NormalizeCo + "splitting", ctx.getTableName(), hri.getRegionNameAsString(), regionSizeMb, String.format("%.3f", avgRegionSize)); - plans.add(new SplitNormalizationPlan(hri, regionSizeMb)); + cumulativePlansSizeMb += getRegionSizeMB(hri); + if (cumulativePlansSizeMb > normalizerConfiguration.getCumulativeSplitPlansSizeLimitMb()) { + return plans; + } else { + plans.add(new SplitNormalizationPlan(hri, regionSizeMb)); + } } } return plans; @@ -484,6 +509,8 @@ private static final class NormalizerConfiguration { private final int mergeMinRegionCount; private final Period mergeMinRegionAge; private final long mergeMinRegionSizeMb; + private final long cumulativeMergePlansSizeLimitMb; + private final long cumulativeSplitPlansSizeLimitMb; private NormalizerConfiguration() { conf = null; @@ -492,6 +519,8 @@ private NormalizerConfiguration() { mergeMinRegionCount = DEFAULT_MERGE_MIN_REGION_COUNT; mergeMinRegionAge = Period.ofDays(DEFAULT_MERGE_MIN_REGION_AGE_DAYS); mergeMinRegionSizeMb = DEFAULT_MERGE_MIN_REGION_SIZE_MB; + cumulativeMergePlansSizeLimitMb = DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB; + cumulativeSplitPlansSizeLimitMb = DEFAULT_CUMULATIVE_SPLIT_SIZE_LIMIT_MB; } private NormalizerConfiguration(final Configuration conf, @@ -502,6 +531,8 @@ private NormalizerConfiguration(final Configuration conf, mergeMinRegionCount = parseMergeMinRegionCount(conf); mergeMinRegionAge = parseMergeMinRegionAge(conf); mergeMinRegionSizeMb = parseMergeMinRegionSizeMb(conf); + cumulativeSplitPlansSizeLimitMb = conf.getLong(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB); + cumulativeMergePlansSizeLimitMb = conf.getLong(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB); logConfigurationUpdated(SPLIT_ENABLED_KEY, currentConfiguration.isSplitEnabled(), splitEnabled); logConfigurationUpdated(MERGE_ENABLED_KEY, currentConfiguration.isMergeEnabled(), @@ -512,6 +543,8 @@ private NormalizerConfiguration(final Configuration conf, currentConfiguration.getMergeMinRegionAge(), mergeMinRegionAge); logConfigurationUpdated(MERGE_MIN_REGION_SIZE_MB_KEY, currentConfiguration.getMergeMinRegionSizeMb(), mergeMinRegionSizeMb); + logConfigurationUpdated(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, currentConfiguration.getCumulativeSplitPlansSizeLimitMb(), cumulativeSplitPlansSizeLimitMb); + logConfigurationUpdated(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, currentConfiguration.getCumulativeMergePlansSizeLimitMb(), cumulativeMergePlansSizeLimitMb); } public Configuration getConf() { @@ -574,6 +607,14 @@ public long getMergeMinRegionSizeMb(NormalizeContext context) { } return mergeMinRegionSizeMb; } + + public long getCumulativeSplitPlansSizeLimitMb() { + return cumulativeSplitPlansSizeLimitMb; + } + + public long getCumulativeMergePlansSizeLimitMb() { + return cumulativeMergePlansSizeLimitMb; + } } /** 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 54d39a9dc00b..392c78847d54 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 @@ -18,6 +18,8 @@ package org.apache.hadoop.hbase.master.normalizer; import static java.lang.String.format; +import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY; +import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY; 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; @@ -30,6 +32,7 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; @@ -607,6 +610,40 @@ public void testNormalizerCannotMergeNonAdjacentRegions() { assertThat(plans, empty()); } + @Test + public void testMergeSizeLimit() { + conf.setBoolean(SPLIT_ENABLED_KEY, false); + conf.setLong(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, 5); + final TableName tableName = name.getTableName(); + final List regionInfos = createRegionInfos(tableName, 6); + final Map regionSizes = + createRegionSizesMap(regionInfos,1, 1, 1, 1, 1, 1); + setupMocksForNormalizer(regionSizes, regionInfos); + when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(2L); + + assertTrue(normalizer.isMergeEnabled()); + assertFalse(normalizer.isSplitEnabled()); + assertThat(normalizer.computePlansForTable(tableDescriptor), + hasSize(2)); // creates 2 merge plans, even though there are 3 otherwise eligible pairs of regions + } + + @Test + public void testSplitSizeLimit() { + conf.setBoolean(MERGE_ENABLED_KEY, false); + conf.setLong(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, 10); + final TableName tableName = name.getTableName(); + final List regionInfos = createRegionInfos(tableName, 4); + final Map regionSizes = + createRegionSizesMap(regionInfos, 3, 3, 3, 3); + setupMocksForNormalizer(regionSizes, regionInfos); + when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(1L); + + assertTrue(normalizer.isSplitEnabled()); + assertFalse(normalizer.isMergeEnabled()); + assertThat(normalizer.computePlansForTable(tableDescriptor), + hasSize(3)); // the plan includes only 3 regions, even though the table has 4 otherwise split-eligible regions + } + @SuppressWarnings("MockitoCast") private void setupMocksForNormalizer(Map regionSizes, List regionInfoList) { From ea41bf28384d7b48506eee4cca5940a2e4493f0c Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Sat, 19 Nov 2022 11:37:50 -0500 Subject: [PATCH 2/6] Use a single setting to limit normalizer plans size --- .../normalizer/MergeNormalizationPlan.java | 9 +++ .../master/normalizer/NormalizationPlan.java | 2 + .../normalizer/SimpleRegionNormalizer.java | 78 +++++++++---------- .../normalizer/SplitNormalizationPlan.java | 5 ++ .../TestSimpleRegionNormalizer.java | 22 +++--- 5 files changed, 62 insertions(+), 54 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java index f89ce749b06f..85c91f527283 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java @@ -58,6 +58,15 @@ public List getNormalizationTargets() { return normalizationTargets; } + @Override + public long getPlanSizeMb() { + long total = 0; + for (NormalizationTarget target : normalizationTargets) { + total += target.getRegionSizeMb(); + } + return total; + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java index 0e0d39d10b1c..de6db2cd554a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java @@ -34,4 +34,6 @@ enum PlanType { /** Returns the type of this plan */ PlanType getType(); + + long getPlanSizeMb(); } 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 388c4adac621..01903de7201e 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 @@ -22,7 +22,6 @@ import java.time.Instant; import java.time.Period; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -80,10 +79,8 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver 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"; static final int DEFAULT_MERGE_MIN_REGION_SIZE_MB = 0; - static final String CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.merge.plans_size_limit.mb"; - static final long DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB = Long.MAX_VALUE; - static final String CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY = "hbase.normalizer.split.plans_size_limit.mb"; - static final long DEFAULT_CUMULATIVE_SPLIT_SIZE_LIMIT_MB = Long.MAX_VALUE; + static final String CUMULATIVE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.plans_size_limit.mb"; + static final long DEFAULT_CUMULATIVE_SIZE_LIMIT_MB = Long.MAX_VALUE; private MasterServices masterServices; private NormalizerConfiguration normalizerConfiguration; @@ -220,7 +217,7 @@ public List computePlansForTable(final TableDescriptor tableD LOG.debug("Computing normalization plan for table: {}, number of regions: {}", table, ctx.getTableRegions().size()); - final List plans = new ArrayList<>(); + List plans = new ArrayList<>(); int splitPlansCount = 0; if (proceedWithSplitPlanning) { List splitPlans = computeSplitNormalizationPlans(ctx); @@ -234,19 +231,13 @@ public List computePlansForTable(final TableDescriptor tableD plans.addAll(mergePlans); } + plans = truncateForSize(plans); + LOG.debug("Computed normalization plans for table {}. Total plans: {}, split plans: {}, " + "merge plans: {}", table, plans.size(), splitPlansCount, mergePlansCount); return plans; } - private long getTotalRegionSizeMB(Collection targets) { - long total = 0; - for (NormalizationTarget target : targets) { - total += target.getRegionSizeMb(); - } - return total; - } - /** Returns size of region in MB and if region is not found than -1 */ private long getRegionSizeMB(RegionInfo hri) { ServerName sn = @@ -370,7 +361,6 @@ private List computeMergeNormalizationPlans(final NormalizeCo final List plans = new LinkedList<>(); final List rangeMembers = new LinkedList<>(); long sumRangeMembersSizeMb; - long cumulativePlansSizeMb = 0; int current = 0; for (int rangeStart = 0; rangeStart < ctx.getTableRegions().size() - 1 && current < ctx.getTableRegions().size();) { @@ -408,12 +398,7 @@ private List computeMergeNormalizationPlans(final NormalizeCo break; } if (rangeMembers.size() > 1) { - cumulativePlansSizeMb += getTotalRegionSizeMB(rangeMembers); - if (cumulativePlansSizeMb > normalizerConfiguration.getCumulativeMergePlansSizeLimitMb()) { - return plans; - } else { - plans.add(new MergeNormalizationPlan.Builder().setTargets(rangeMembers).build()); - } + plans.add(new MergeNormalizationPlan.Builder().setTargets(rangeMembers).build()); } } return plans; @@ -441,7 +426,6 @@ private List computeSplitNormalizationPlans(final NormalizeCo LOG.debug("Table {}, average region size: {} MB", ctx.getTableName(), String.format("%.3f", avgRegionSize)); - long cumulativePlansSizeMb = 0; final List plans = new ArrayList<>(); for (final RegionInfo hri : ctx.getTableRegions()) { if (skipForSplit(ctx.getRegionStates().getRegionState(hri), hri)) { @@ -454,12 +438,7 @@ private List computeSplitNormalizationPlans(final NormalizeCo + "splitting", ctx.getTableName(), hri.getRegionNameAsString(), regionSizeMb, String.format("%.3f", avgRegionSize)); - cumulativePlansSizeMb += getRegionSizeMB(hri); - if (cumulativePlansSizeMb > normalizerConfiguration.getCumulativeSplitPlansSizeLimitMb()) { - return plans; - } else { - plans.add(new SplitNormalizationPlan(hri, regionSizeMb)); - } + plans.add(new SplitNormalizationPlan(hri, regionSizeMb)); } } return plans; @@ -489,6 +468,27 @@ private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerCo return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx); } + private List truncateForSize(List plans) { + if ( + normalizerConfiguration.getCumulativePlansSizeLimitMb() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB + ) { + // If we are going to truncate our list of plans, shuffle the split and merge plans together + // so that the merge plans, which are listed last, are not starved out. + List maybeTruncatedPlans = new ArrayList<>(); + Collections.shuffle(plans); + long cumulativeSizeMb = 0; + for (NormalizationPlan plan : plans) { + cumulativeSizeMb += plan.getPlanSizeMb(); + if (cumulativeSizeMb < normalizerConfiguration.getCumulativePlansSizeLimitMb()) { + maybeTruncatedPlans.add(plan); + } + } + return maybeTruncatedPlans; + } else { + return plans; + } + } + private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue, final Object... args) { final boolean value = predicate.getAsBoolean(); @@ -509,8 +509,7 @@ private static final class NormalizerConfiguration { private final int mergeMinRegionCount; private final Period mergeMinRegionAge; private final long mergeMinRegionSizeMb; - private final long cumulativeMergePlansSizeLimitMb; - private final long cumulativeSplitPlansSizeLimitMb; + private final long cumulativePlansSizeLimitMb; private NormalizerConfiguration() { conf = null; @@ -519,8 +518,7 @@ private NormalizerConfiguration() { mergeMinRegionCount = DEFAULT_MERGE_MIN_REGION_COUNT; mergeMinRegionAge = Period.ofDays(DEFAULT_MERGE_MIN_REGION_AGE_DAYS); mergeMinRegionSizeMb = DEFAULT_MERGE_MIN_REGION_SIZE_MB; - cumulativeMergePlansSizeLimitMb = DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB; - cumulativeSplitPlansSizeLimitMb = DEFAULT_CUMULATIVE_SPLIT_SIZE_LIMIT_MB; + cumulativePlansSizeLimitMb = DEFAULT_CUMULATIVE_SIZE_LIMIT_MB; } private NormalizerConfiguration(final Configuration conf, @@ -531,8 +529,8 @@ private NormalizerConfiguration(final Configuration conf, mergeMinRegionCount = parseMergeMinRegionCount(conf); mergeMinRegionAge = parseMergeMinRegionAge(conf); mergeMinRegionSizeMb = parseMergeMinRegionSizeMb(conf); - cumulativeSplitPlansSizeLimitMb = conf.getLong(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB); - cumulativeMergePlansSizeLimitMb = conf.getLong(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_MERGE_SIZE_LIMIT_MB); + cumulativePlansSizeLimitMb = + conf.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB); logConfigurationUpdated(SPLIT_ENABLED_KEY, currentConfiguration.isSplitEnabled(), splitEnabled); logConfigurationUpdated(MERGE_ENABLED_KEY, currentConfiguration.isMergeEnabled(), @@ -543,8 +541,8 @@ private NormalizerConfiguration(final Configuration conf, currentConfiguration.getMergeMinRegionAge(), mergeMinRegionAge); logConfigurationUpdated(MERGE_MIN_REGION_SIZE_MB_KEY, currentConfiguration.getMergeMinRegionSizeMb(), mergeMinRegionSizeMb); - logConfigurationUpdated(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, currentConfiguration.getCumulativeSplitPlansSizeLimitMb(), cumulativeSplitPlansSizeLimitMb); - logConfigurationUpdated(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, currentConfiguration.getCumulativeMergePlansSizeLimitMb(), cumulativeMergePlansSizeLimitMb); + logConfigurationUpdated(CUMULATIVE_SIZE_LIMIT_MB_KEY, + currentConfiguration.getCumulativePlansSizeLimitMb(), cumulativePlansSizeLimitMb); } public Configuration getConf() { @@ -608,12 +606,8 @@ public long getMergeMinRegionSizeMb(NormalizeContext context) { return mergeMinRegionSizeMb; } - public long getCumulativeSplitPlansSizeLimitMb() { - return cumulativeSplitPlansSizeLimitMb; - } - - public long getCumulativeMergePlansSizeLimitMb() { - return cumulativeMergePlansSizeLimitMb; + public long getCumulativePlansSizeLimitMb() { + return cumulativePlansSizeLimitMb; } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java index 6068eccdea17..5cbddda1483f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java @@ -45,6 +45,11 @@ public NormalizationTarget getSplitTarget() { return splitTarget; } + @Override + public long getPlanSizeMb() { + return splitTarget.getRegionSizeMb(); + } + @Override public String toString() { return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) 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 392c78847d54..c722b6d3694f 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 @@ -18,8 +18,7 @@ package org.apache.hadoop.hbase.master.normalizer; import static java.lang.String.format; -import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY; -import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY; +import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_SIZE_LIMIT_MB_KEY; 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; @@ -613,35 +612,34 @@ public void testNormalizerCannotMergeNonAdjacentRegions() { @Test public void testMergeSizeLimit() { conf.setBoolean(SPLIT_ENABLED_KEY, false); - conf.setLong(CUMULATIVE_MERGE_SIZE_LIMIT_MB_KEY, 5); + conf.setLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, 5); final TableName tableName = name.getTableName(); final List regionInfos = createRegionInfos(tableName, 6); - final Map regionSizes = - createRegionSizesMap(regionInfos,1, 1, 1, 1, 1, 1); + final Map regionSizes = createRegionSizesMap(regionInfos, 1, 1, 1, 1, 1, 1); setupMocksForNormalizer(regionSizes, regionInfos); when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(2L); assertTrue(normalizer.isMergeEnabled()); assertFalse(normalizer.isSplitEnabled()); - assertThat(normalizer.computePlansForTable(tableDescriptor), - hasSize(2)); // creates 2 merge plans, even though there are 3 otherwise eligible pairs of regions + // creates 2 merge plans, even though there are 3 otherwise eligible pairs of regions + assertThat(normalizer.computePlansForTable(tableDescriptor), hasSize(2)); } @Test public void testSplitSizeLimit() { conf.setBoolean(MERGE_ENABLED_KEY, false); - conf.setLong(CUMULATIVE_SPLIT_SIZE_LIMIT_MB_KEY, 10); + conf.setLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, 10); final TableName tableName = name.getTableName(); final List regionInfos = createRegionInfos(tableName, 4); - final Map regionSizes = - createRegionSizesMap(regionInfos, 3, 3, 3, 3); + final Map regionSizes = createRegionSizesMap(regionInfos, 3, 3, 3, 3); setupMocksForNormalizer(regionSizes, regionInfos); when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(1L); assertTrue(normalizer.isSplitEnabled()); assertFalse(normalizer.isMergeEnabled()); - assertThat(normalizer.computePlansForTable(tableDescriptor), - hasSize(3)); // the plan includes only 3 regions, even though the table has 4 otherwise split-eligible regions + // the plan includes only 3 regions, even though the table has 4 otherwise split-eligible + // regions + assertThat(normalizer.computePlansForTable(tableDescriptor), hasSize(3)); } @SuppressWarnings("MockitoCast") From 7c2025b715a744603be8b25ef1183c99c1131600 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Mon, 21 Nov 2022 12:31:08 -0500 Subject: [PATCH 3/6] PR feedback: move plans truncation into RegionNormalizerWorker --- .../normalizer/RegionNormalizerWorker.java | 29 ++++++++++++++- .../normalizer/SimpleRegionNormalizer.java | 35 ++++++++----------- .../TestRegionNormalizerWorker.java | 30 ++++++++++++++++ .../TestSimpleRegionNormalizer.java | 32 ++++++----------- 4 files changed, 82 insertions(+), 44 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java index 8890c2aba798..8ba096fe7f47 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.time.Duration; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.apache.hadoop.conf.Configuration; @@ -53,6 +54,9 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab "hbase.normalizer.throughput.max_bytes_per_sec"; private static final long RATE_UNLIMITED_BYTES = 1_000_000_000_000L; // 1TB/sec + static final String CUMULATIVE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.plans_size_limit.mb"; + static final long DEFAULT_CUMULATIVE_SIZE_LIMIT_MB = Long.MAX_VALUE; + private final MasterServices masterServices; private final RegionNormalizer regionNormalizer; private final RegionNormalizerWorkQueue workQueue; @@ -62,6 +66,7 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab private final boolean defaultNormalizerTableLevel; private long splitPlanCount; private long mergePlanCount; + private long cumulativePlansSizeLimitMb; RegionNormalizerWorker(final Configuration configuration, final MasterServices masterServices, final RegionNormalizer regionNormalizer, final RegionNormalizerWorkQueue workQueue) { @@ -73,6 +78,8 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab this.mergePlanCount = 0; this.rateLimiter = loadRateLimiter(configuration); this.defaultNormalizerTableLevel = extractDefaultNormalizerValue(configuration); + this.cumulativePlansSizeLimitMb = + configuration.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB); } private boolean extractDefaultNormalizerValue(final Configuration configuration) { @@ -207,7 +214,10 @@ private List calculatePlans(final TableName tableName) { return Collections.emptyList(); } - final List plans = regionNormalizer.computePlansForTable(tblDesc); + List plans = regionNormalizer.computePlansForTable(tblDesc); + + plans = truncateForSize(plans); + if (CollectionUtils.isEmpty(plans)) { LOG.debug("No normalization required for table {}.", tableName); return Collections.emptyList(); @@ -215,6 +225,23 @@ private List calculatePlans(final TableName tableName) { return plans; } + private List truncateForSize(List plans) { + if (cumulativePlansSizeLimitMb != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB) { + List maybeTruncatedPlans = new ArrayList<>(plans.size()); + long cumulativeSizeMb = 0; + for (NormalizationPlan plan : plans) { + cumulativeSizeMb += plan.getPlanSizeMb(); + if (cumulativeSizeMb > cumulativePlansSizeLimitMb) { + break; + } + maybeTruncatedPlans.add(plan); + } + return maybeTruncatedPlans; + } else { + return plans; + } + } + private void submitPlans(final List plans) { // as of this writing, `plan.submit()` is non-blocking and uses Async Admin APIs to submit // task, so there's no artificial rate-limiting of merge/split requests due to this serial loop. 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 01903de7201e..151475596e0b 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 @@ -217,7 +217,7 @@ public List computePlansForTable(final TableDescriptor tableD LOG.debug("Computing normalization plan for table: {}, number of regions: {}", table, ctx.getTableRegions().size()); - List plans = new ArrayList<>(); + final List plans = new ArrayList<>(); int splitPlansCount = 0; if (proceedWithSplitPlanning) { List splitPlans = computeSplitNormalizationPlans(ctx); @@ -231,7 +231,13 @@ public List computePlansForTable(final TableDescriptor tableD plans.addAll(mergePlans); } - plans = truncateForSize(plans); + if ( + normalizerConfiguration.getCumulativePlansSizeLimitMb() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB + ) { + // If we are going to truncate our list of plans, shuffle the split and merge plans together + // so that the merge plans, which are listed last, are not starved out. + shuffleNormalizationPlans(plans); + } LOG.debug("Computed normalization plans for table {}. Total plans: {}, split plans: {}, " + "merge plans: {}", table, plans.size(), splitPlansCount, mergePlansCount); @@ -468,25 +474,12 @@ private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerCo return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx); } - private List truncateForSize(List plans) { - if ( - normalizerConfiguration.getCumulativePlansSizeLimitMb() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB - ) { - // If we are going to truncate our list of plans, shuffle the split and merge plans together - // so that the merge plans, which are listed last, are not starved out. - List maybeTruncatedPlans = new ArrayList<>(); - Collections.shuffle(plans); - long cumulativeSizeMb = 0; - for (NormalizationPlan plan : plans) { - cumulativeSizeMb += plan.getPlanSizeMb(); - if (cumulativeSizeMb < normalizerConfiguration.getCumulativePlansSizeLimitMb()) { - maybeTruncatedPlans.add(plan); - } - } - return maybeTruncatedPlans; - } else { - return plans; - } + /** + * This very simple method exists so we can verify it was called in a unit test. Visible for + * testing. + */ + void shuffleNormalizationPlans(List plans) { + Collections.shuffle(plans); } private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerWorker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerWorker.java index 91f5fb3ced7b..c5f0a201cb0c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerWorker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestRegionNormalizerWorker.java @@ -204,6 +204,36 @@ public void testRateLimit() throws Exception { Duration.ofNanos(endTime - startTime), greaterThanOrEqualTo(Duration.ofSeconds(5))); } + @Test + public void testPlansSizeLimit() throws Exception { + final TableName tn = tableName.getTableName(); + final TableDescriptor tnDescriptor = + TableDescriptorBuilder.newBuilder(tn).setNormalizationEnabled(true).build(); + final RegionInfo splitRegionInfo = RegionInfoBuilder.newBuilder(tn).build(); + final RegionInfo mergeRegionInfo1 = RegionInfoBuilder.newBuilder(tn).build(); + final RegionInfo mergeRegionInfo2 = RegionInfoBuilder.newBuilder(tn).build(); + when(masterServices.getTableDescriptors().get(tn)).thenReturn(tnDescriptor); + when(masterServices.splitRegion(any(), any(), anyLong(), anyLong())).thenReturn(1L); + when(masterServices.mergeRegions(any(), anyBoolean(), anyLong(), anyLong())).thenReturn(1L); + when(regionNormalizer.computePlansForTable(tnDescriptor)).thenReturn(Arrays.asList( + new SplitNormalizationPlan(splitRegionInfo, 2), new MergeNormalizationPlan.Builder() + .addTarget(mergeRegionInfo1, 1).addTarget(mergeRegionInfo2, 2).build(), + new SplitNormalizationPlan(splitRegionInfo, 1))); + + final Configuration conf = testingUtility.getConfiguration(); + conf.setLong(RegionNormalizerWorker.CUMULATIVE_SIZE_LIMIT_MB_KEY, 5); + + final RegionNormalizerWorker worker = new RegionNormalizerWorker( + testingUtility.getConfiguration(), masterServices, regionNormalizer, queue); + workerPool.submit(worker); + queue.put(tn); + + assertThatEventually("worker should process first split plan, but not second", + worker::getSplitPlanCount, comparesEqualTo(1L)); + assertThatEventually("worker should process merge plan", worker::getMergePlanCount, + comparesEqualTo(1L)); + } + /** * Repeatedly evaluates {@code matcher} against the result of calling {@code actualSupplier} until * the matcher succeeds or the timeout period of 30 seconds is exhausted. 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 c722b6d3694f..2ed15080b4e6 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 @@ -38,7 +38,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.time.Instant; @@ -610,36 +614,20 @@ public void testNormalizerCannotMergeNonAdjacentRegions() { } @Test - public void testMergeSizeLimit() { - conf.setBoolean(SPLIT_ENABLED_KEY, false); - conf.setLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, 5); - final TableName tableName = name.getTableName(); - final List regionInfos = createRegionInfos(tableName, 6); - final Map regionSizes = createRegionSizesMap(regionInfos, 1, 1, 1, 1, 1, 1); - setupMocksForNormalizer(regionSizes, regionInfos); - when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(2L); - - assertTrue(normalizer.isMergeEnabled()); - assertFalse(normalizer.isSplitEnabled()); - // creates 2 merge plans, even though there are 3 otherwise eligible pairs of regions - assertThat(normalizer.computePlansForTable(tableDescriptor), hasSize(2)); - } - - @Test - public void testSplitSizeLimit() { - conf.setBoolean(MERGE_ENABLED_KEY, false); + public void testSizeLimitShufflesPlans() { conf.setLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, 10); final TableName tableName = name.getTableName(); final List regionInfos = createRegionInfos(tableName, 4); final Map regionSizes = createRegionSizesMap(regionInfos, 3, 3, 3, 3); setupMocksForNormalizer(regionSizes, regionInfos); when(tableDescriptor.getNormalizerTargetRegionSize()).thenReturn(1L); + normalizer = spy(normalizer); assertTrue(normalizer.isSplitEnabled()); - assertFalse(normalizer.isMergeEnabled()); - // the plan includes only 3 regions, even though the table has 4 otherwise split-eligible - // regions - assertThat(normalizer.computePlansForTable(tableDescriptor), hasSize(3)); + assertTrue(normalizer.isMergeEnabled()); + List computedPlans = normalizer.computePlansForTable(tableDescriptor); + assertThat(computedPlans, hasSize(4)); + verify(normalizer, times(1)).shuffleNormalizationPlans(anyList()); } @SuppressWarnings("MockitoCast") From f29755f54b1d4df9d6931e5f5c111448dded34f4 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Mon, 21 Nov 2022 14:06:12 -0500 Subject: [PATCH 4/6] Add logging --- .../normalizer/RegionNormalizerWorker.java | 25 +++++++++++++++---- .../normalizer/SimpleRegionNormalizer.java | 8 +++--- .../TestSimpleRegionNormalizer.java | 2 +- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java index 8ba096fe7f47..87dddec213e0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; @@ -66,7 +67,7 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab private final boolean defaultNormalizerTableLevel; private long splitPlanCount; private long mergePlanCount; - private long cumulativePlansSizeLimitMb; + private final AtomicLong cumulativePlansSizeLimitMb; RegionNormalizerWorker(final Configuration configuration, final MasterServices masterServices, final RegionNormalizer regionNormalizer, final RegionNormalizerWorkQueue workQueue) { @@ -78,8 +79,8 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab this.mergePlanCount = 0; this.rateLimiter = loadRateLimiter(configuration); this.defaultNormalizerTableLevel = extractDefaultNormalizerValue(configuration); - this.cumulativePlansSizeLimitMb = - configuration.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB); + this.cumulativePlansSizeLimitMb = new AtomicLong( + configuration.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB)); } private boolean extractDefaultNormalizerValue(final Configuration configuration) { @@ -103,9 +104,20 @@ public void deregisterChildren(ConfigurationManager manager) { } } + private static long logLongConfigurationUpdated(final String key, final long oldValue, + final long newValue) { + if (oldValue != newValue) { + LOG.info("Updated configuration for key '{}' from {} to {}", key, oldValue, newValue); + } + return newValue; + } + @Override public void onConfigurationChange(Configuration conf) { rateLimiter.setRate(loadRateLimit(conf)); + cumulativePlansSizeLimitMb.set( + logLongConfigurationUpdated(CUMULATIVE_SIZE_LIMIT_MB_KEY, cumulativePlansSizeLimitMb.get(), + conf.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB))); } private static RateLimiter loadRateLimiter(final Configuration configuration) { @@ -226,12 +238,15 @@ private List calculatePlans(final TableName tableName) { } private List truncateForSize(List plans) { - if (cumulativePlansSizeLimitMb != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB) { + if (cumulativePlansSizeLimitMb.get() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB) { List maybeTruncatedPlans = new ArrayList<>(plans.size()); long cumulativeSizeMb = 0; for (NormalizationPlan plan : plans) { cumulativeSizeMb += plan.getPlanSizeMb(); - if (cumulativeSizeMb > cumulativePlansSizeLimitMb) { + if (cumulativeSizeMb > cumulativePlansSizeLimitMb.get()) { + LOG.debug( + "Truncating list of normalization plans that RegionNormalizerWorker will process because of {}. Original list had size {}, new list has size {}.", + CUMULATIVE_SIZE_LIMIT_MB_KEY, plans.size(), maybeTruncatedPlans.size()); break; } maybeTruncatedPlans.add(plan); 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 151475596e0b..dfae394b75a6 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 @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hbase.master.normalizer; +import static org.apache.hadoop.hbase.master.normalizer.RegionNormalizerWorker.CUMULATIVE_SIZE_LIMIT_MB_KEY; +import static org.apache.hadoop.hbase.master.normalizer.RegionNormalizerWorker.DEFAULT_CUMULATIVE_SIZE_LIMIT_MB; import static org.apache.hbase.thirdparty.org.apache.commons.collections4.CollectionUtils.isEmpty; import java.time.Instant; @@ -79,8 +81,6 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver 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"; static final int DEFAULT_MERGE_MIN_REGION_SIZE_MB = 0; - static final String CUMULATIVE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.plans_size_limit.mb"; - static final long DEFAULT_CUMULATIVE_SIZE_LIMIT_MB = Long.MAX_VALUE; private MasterServices masterServices; private NormalizerConfiguration normalizerConfiguration; @@ -534,8 +534,6 @@ private NormalizerConfiguration(final Configuration conf, currentConfiguration.getMergeMinRegionAge(), mergeMinRegionAge); logConfigurationUpdated(MERGE_MIN_REGION_SIZE_MB_KEY, currentConfiguration.getMergeMinRegionSizeMb(), mergeMinRegionSizeMb); - logConfigurationUpdated(CUMULATIVE_SIZE_LIMIT_MB_KEY, - currentConfiguration.getCumulativePlansSizeLimitMb(), cumulativePlansSizeLimitMb); } public Configuration getConf() { @@ -599,7 +597,7 @@ public long getMergeMinRegionSizeMb(NormalizeContext context) { return mergeMinRegionSizeMb; } - public long getCumulativePlansSizeLimitMb() { + private long getCumulativePlansSizeLimitMb() { return cumulativePlansSizeLimitMb; } } 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 2ed15080b4e6..5dba036bb705 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 @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.master.normalizer; import static java.lang.String.format; -import static org.apache.hadoop.hbase.master.normalizer.SimpleRegionNormalizer.CUMULATIVE_SIZE_LIMIT_MB_KEY; +import static org.apache.hadoop.hbase.master.normalizer.RegionNormalizerWorker.CUMULATIVE_SIZE_LIMIT_MB_KEY; 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; From bd0ad01234e5315872ff8087496b7d94de5ff77b Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Mon, 21 Nov 2022 16:41:05 -0500 Subject: [PATCH 5/6] Log more information --- .../normalizer/RegionNormalizerWorker.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java index 87dddec213e0..4a113493662c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java @@ -240,16 +240,20 @@ private List calculatePlans(final TableName tableName) { private List truncateForSize(List plans) { if (cumulativePlansSizeLimitMb.get() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB) { List maybeTruncatedPlans = new ArrayList<>(plans.size()); - long cumulativeSizeMb = 0; + long totalCumulativeSizeMb = 0; + long truncatedCumulativeSizeMb = 0; for (NormalizationPlan plan : plans) { - cumulativeSizeMb += plan.getPlanSizeMb(); - if (cumulativeSizeMb > cumulativePlansSizeLimitMb.get()) { - LOG.debug( - "Truncating list of normalization plans that RegionNormalizerWorker will process because of {}. Original list had size {}, new list has size {}.", - CUMULATIVE_SIZE_LIMIT_MB_KEY, plans.size(), maybeTruncatedPlans.size()); - break; + totalCumulativeSizeMb += plan.getPlanSizeMb(); + if (totalCumulativeSizeMb <= cumulativePlansSizeLimitMb.get()) { + truncatedCumulativeSizeMb += plan.getPlanSizeMb(); + maybeTruncatedPlans.add(plan); } - maybeTruncatedPlans.add(plan); + } + if (maybeTruncatedPlans.size() != plans.size()) { + LOG.debug( + "Truncating list of normalization plans that RegionNormalizerWorker will process because of {}. Original list had {} plan(s), new list has {} plan(s). Original list covered regions with cumulative size {} mb, new list covers regions with cumulative size {} mb.", + CUMULATIVE_SIZE_LIMIT_MB_KEY, plans.size(), maybeTruncatedPlans.size(), + totalCumulativeSizeMb, truncatedCumulativeSizeMb); } return maybeTruncatedPlans; } else { From 0aad51799d1f04cc3fad3b278e466b18cd75ddac Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Tue, 22 Nov 2022 08:15:50 -0500 Subject: [PATCH 6/6] shorter line --- .../hbase/master/normalizer/RegionNormalizerWorker.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java index 4a113493662c..0a701cd3ad63 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java @@ -251,7 +251,10 @@ private List truncateForSize(List plans) { } if (maybeTruncatedPlans.size() != plans.size()) { LOG.debug( - "Truncating list of normalization plans that RegionNormalizerWorker will process because of {}. Original list had {} plan(s), new list has {} plan(s). Original list covered regions with cumulative size {} mb, new list covers regions with cumulative size {} mb.", + "Truncating list of normalization plans that RegionNormalizerWorker will process " + + "because of {}. Original list had {} plan(s), new list has {} plan(s). " + + "Original list covered regions with cumulative size {} mb, " + + "new list covers regions with cumulative size {} mb.", CUMULATIVE_SIZE_LIMIT_MB_KEY, plans.size(), maybeTruncatedPlans.size(), totalCumulativeSizeMb, truncatedCumulativeSizeMb); }