[hbase] branch branch-2 updated: HBASE-20289 Fix comparator for NormalizationPlan

[nd...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ndimiduk pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new 26617df  HBASE-20289 Fix comparator for NormalizationPlan
26617df is described below

commit 26617df3735839b9cd798ebdbad50bfa938b58de
Author: Yuki Tawara <yk...@gmail.com>
AuthorDate: Tue Mar 27 01:22:53 2018 +0900

    HBASE-20289 Fix comparator for NormalizationPlan
    
    Signed-off-by: tedyu <yu...@gmail.com>
---
 .../master/normalizer/SimpleRegionNormalizer.java  | 25 ++++++++++++++--------
 .../normalizer/TestSimpleRegionNormalizer.java     | 17 +++++++++++++++
 2 files changed, 33 insertions(+), 9 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 435f1ca..8d566f0 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
@@ -92,20 +92,27 @@ public class SimpleRegionNormalizer implements RegionNormalizer {
     return skippedCount[type.ordinal()];
   }
 
-  // Comparator that gives higher priority to region Split plan
-  private Comparator<NormalizationPlan> planComparator =
-      new Comparator<NormalizationPlan>() {
+  /**
+   * Comparator class that gives higher priority to region Split plan.
+   */
+  static class PlanComparator implements Comparator<NormalizationPlan> {
     @Override
-    public int compare(NormalizationPlan plan, NormalizationPlan plan2) {
-      if (plan instanceof SplitNormalizationPlan) {
+    public int compare(NormalizationPlan plan1, NormalizationPlan plan2) {
+      boolean plan1IsSplit = plan1 instanceof SplitNormalizationPlan;
+      boolean plan2IsSplit = plan2 instanceof SplitNormalizationPlan;
+      if (plan1IsSplit && plan2IsSplit) {
+        return 0;
+      } else if (plan1IsSplit) {
         return -1;
-      }
-      if (plan2 instanceof SplitNormalizationPlan) {
+      } else if (plan2IsSplit) {
         return 1;
+      } else {
+        return 0;
       }
-      return 0;
     }
-  };
+  }
+
+  private Comparator<NormalizationPlan> planComparator = new PlanComparator();
 
   /**
    * Computes next most "urgent" normalization action on the table.
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 d2123fb..743ec2b 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
@@ -24,6 +24,7 @@ import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -82,6 +83,22 @@ public class TestSimpleRegionNormalizer {
   }
 
   @Test
+  public void testPlanComparator() {
+    Comparator<NormalizationPlan> comparator = new SimpleRegionNormalizer.PlanComparator();
+    NormalizationPlan splitPlan1 = new SplitNormalizationPlan(null, null);
+    NormalizationPlan splitPlan2 = new SplitNormalizationPlan(null, null);
+    NormalizationPlan mergePlan1 = new MergeNormalizationPlan(null, null);
+    NormalizationPlan mergePlan2 = new MergeNormalizationPlan(null, null);
+
+    assertTrue(comparator.compare(splitPlan1, splitPlan2) == 0);
+    assertTrue(comparator.compare(splitPlan2, splitPlan1) == 0);
+    assertTrue(comparator.compare(mergePlan1, mergePlan2) == 0);
+    assertTrue(comparator.compare(mergePlan2, mergePlan1) == 0);
+    assertTrue(comparator.compare(splitPlan1, mergePlan1) < 0);
+    assertTrue(comparator.compare(mergePlan1, splitPlan1) > 0);
+  }
+
+  @Test
   public void testNoNormalizationForMetaTable() throws HBaseIOException {
     TableName testTable = TableName.META_TABLE_NAME;
     List<RegionInfo> RegionInfo = new ArrayList<>();