[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer

[GitBox <gi...@apache.org>]

bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1027120506


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -464,6 +468,27 @@ private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerCo
     return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
   }
 
+  private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> 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<NormalizationPlan> maybeTruncatedPlans = new ArrayList<>();
+      Collections.shuffle(plans);
+      long cumulativeSizeMb = 0;
+      for (NormalizationPlan plan : plans) {
+        cumulativeSizeMb += plan.getPlanSizeMb();
+        if (cumulativeSizeMb < normalizerConfiguration.getCumulativePlansSizeLimitMb()) {
+          maybeTruncatedPlans.add(plan);

Review Comment:
   Could you move this down a line and instead have this if block return early? Will just cut a little cpu when there are very large plans



##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java:
##########
@@ -607,6 +609,39 @@ public void testNormalizerCannotMergeNonAdjacentRegions() {
     assertThat(plans, empty());
   }
 
+  @Test
+  public void testMergeSizeLimit() {

Review Comment:
   Can you add a test for the shuffle stuff? Just proving that the limit doesn't starve merges like we hope



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org