HBASE-20289 Fix comparator for NormalizationPlan

Signed-off-by: tedyu <yuzhihong@gmail.com>
apache · Mar 10, 2020 · 26617df · 26617df
1 parent a6700bc
commit 26617df
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 9 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
@@ -92,20 +92,27 @@ public long getSkippedCount(NormalizationPlan.PlanType type) {
     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/...r/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java b/...r/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java
@@ -24,6 +24,7 @@
 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;
@@ -81,6 +82,22 @@ public static void beforeAllTests() throws Exception {
     normalizer = new SimpleRegionNormalizer();
   }
 
+  @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;