From 26617df3735839b9cd798ebdbad50bfa938b58de Mon Sep 17 00:00:00 2001
From: Yuki Tawara <yktawara@gmail.com>
Date: Tue, 27 Mar 2018 01:22:53 +0900
Subject: [PATCH] HBASE-20289 Fix comparator for NormalizationPlan

Signed-off-by: tedyu <yuzhihong@gmail.com>
---
 .../normalizer/SimpleRegionNormalizer.java    | 25 ++++++++++++-------
 .../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 435f1ca791c..8d566f081fb 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 d2123fb44f6..743ec2be485 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;
@@ -81,6 +82,22 @@ public class TestSimpleRegionNormalizer {
     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;