hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject hbase git commit: HBASE-19276 RegionPlan should correctly implement equals and hashCode
Date Fri, 17 Nov 2017 02:09:22 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1.4 2200397fc -> 12d7f0831


HBASE-19276 RegionPlan should correctly implement equals and hashCode

Andrew's patch that adds equals and hash but revamping compare also.

Signed-off-by: Andrew Purtell <apurtell@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/12d7f083
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/12d7f083
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/12d7f083

Branch: refs/heads/branch-1.4
Commit: 12d7f08317859bc37303038cb2eed7702de24eae
Parents: 2200397
Author: Michael Stack <stack@apache.org>
Authored: Wed Nov 15 23:23:28 2017 -0800
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Thu Nov 16 18:09:17 2017 -0800

----------------------------------------------------------------------
 .../apache/hadoop/hbase/master/RegionPlan.java  | 78 ++++++++++++++++---
 .../hadoop/hbase/master/TestRegionPlan.java     | 82 +++++++++++++++-----
 2 files changed, 130 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/12d7f083/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java
index cd6b313..0d84bd9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -43,15 +43,11 @@ public class RegionPlan implements Comparable<RegionPlan> {
   private ServerName dest;
 
   public static class RegionPlanComparator implements Comparator<RegionPlan>, Serializable
{
-
     private static final long serialVersionUID = 4213207330485734853L;
 
     @Override
     public int compare(RegionPlan l, RegionPlan r) {
-      long diff = r.getRegionInfo().getRegionId() - l.getRegionInfo().getRegionId();
-      if (diff < 0) return -1;
-      if (diff > 0) return 1;
-      return 0;
+      return RegionPlan.compareTo(l, r);
     }
   }
 
@@ -109,16 +105,50 @@ public class RegionPlan implements Comparable<RegionPlan> {
 
   /**
    * Compare the region info.
-   * @param o region plan you are comparing against
+   * @param other region plan you are comparing against
    */
   @Override
-  public int compareTo(RegionPlan o) {
-    return getRegionName().compareTo(o.getRegionName());
+  public int compareTo(RegionPlan other) {
+    return compareTo(this, other);
+  }
+
+  private static int compareTo(RegionPlan left, RegionPlan right) {
+    int result = compareServerName(left.source, right.source);
+    if (result != 0) {
+      return result;
+    }
+    if (left.hri == null) {
+      if (right.hri != null) {
+        return -1;
+      }
+    } else if (right.hri == null) {
+      return +1;
+    } else {
+      result = left.hri.compareTo(right.hri);
+    }
+    if (result != 0) {
+      return result;
+    }
+    return compareServerName(left.dest, right.dest);
+  }
+
+  private static int compareServerName(ServerName left, ServerName right) {
+    if (left == null) {
+      return right == null? 0: -1;
+    } else if (right == null) {
+      return +1;
+    }
+    return left.compareTo(right);
   }
 
   @Override
   public int hashCode() {
-    return getRegionName().hashCode();
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + ((dest == null) ? 0 : dest.hashCode());
+    result = prime * result + ((hri == null) ? 0 : hri.hashCode());
+    result = prime * result + ((source == null) ? 0 : source.hashCode());
+    return result;
   }
 
   @Override
@@ -126,11 +156,35 @@ public class RegionPlan implements Comparable<RegionPlan> {
     if (this == obj) {
       return true;
     }
-    if (obj == null || getClass() != obj.getClass()) {
+    if (obj == null) {
+      return false;
+    }
+    if (getClass() != obj.getClass()) {
       return false;
     }
     RegionPlan other = (RegionPlan) obj;
-    return compareTo(other) == 0;
+    if (dest == null) {
+      if (other.dest != null) {
+        return false;
+      }
+    } else if (!dest.equals(other.dest)) {
+      return false;
+    }
+    if (hri == null) {
+      if (other.hri != null) {
+        return false;
+      }
+    } else if (!hri.equals(other.hri)) {
+      return false;
+    }
+    if (source == null) {
+      if (other.source != null) {
+        return false;
+      }
+    } else if (!source.equals(other.source)) {
+      return false;
+    }
+    return true;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/12d7f083/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlan.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlan.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlan.java
index e5b1ca5..3f1cd89 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlan.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlan.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -17,36 +17,82 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static junit.framework.TestCase.assertFalse;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 
-import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.TableName;
+import org.junit.rules.TestName;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 @Category(SmallTests.class)
 public class TestRegionPlan {
+  private final ServerName SRC = ServerName.valueOf("source", 1234, 2345);
+  private final ServerName DEST = ServerName.valueOf("dest", 1234, 2345);
+  @Rule
+  public TestName name = new TestName();
+
   @Test
-  public void test() {
-    HRegionInfo hri = new HRegionInfo(TableName.valueOf("table"));
-    ServerName source = ServerName.valueOf("source", 1234, 2345);
-    ServerName dest = ServerName.valueOf("dest", 1234, 2345);
-    
-    // Identiy equality
-    RegionPlan plan = new RegionPlan(hri, source, dest);
-    assertEquals(plan.hashCode(), new RegionPlan(hri, source, dest).hashCode());
-    assertEquals(plan, new RegionPlan(hri, source, dest));
-
-    // Source and destination not used for equality
-    assertEquals(plan.hashCode(), new RegionPlan(hri, dest, source).hashCode());
-    assertEquals(plan, new RegionPlan(hri, dest, source));
+  public void testCompareTo() {
+    HRegionInfo hri = new HRegionInfo(TableName.valueOf(name.getMethodName()));
+    RegionPlan a = new RegionPlan(hri, null, null);
+    RegionPlan b = new RegionPlan(hri, null, null);
+    assertEquals(0, a.compareTo(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, null, null);
+    assertEquals(1, a.compareTo(b));
+    a = new RegionPlan(hri, null, null);
+    b = new RegionPlan(hri, SRC, null);
+    assertEquals(-1, a.compareTo(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, SRC, null);
+    assertEquals(0, a.compareTo(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, SRC, DEST);
+    assertEquals(-1, a.compareTo(b));
+    a = new RegionPlan(hri, SRC, DEST);
+    b = new RegionPlan(hri, SRC, DEST);
+    assertEquals(0, a.compareTo(b));
+  }
+
+  @Test
+  public void testEqualsWithNulls() {
+    HRegionInfo hri = new HRegionInfo(TableName.valueOf(name.getMethodName()));
+    RegionPlan a = new RegionPlan(hri, null, null);
+    RegionPlan b = new RegionPlan(hri, null, null);
+    assertTrue(a.equals(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, null, null);
+    assertFalse(a.equals(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, SRC, null);
+    assertTrue(a.equals(b));
+    a = new RegionPlan(hri, SRC, null);
+    b = new RegionPlan(hri, SRC, DEST);
+    assertFalse(a.equals(b));
+  }
+
+  @Test
+  public void testEquals() {
+    HRegionInfo hri = new HRegionInfo(TableName.valueOf(name.getMethodName()));
+
+    // Identity equality
+    RegionPlan plan = new RegionPlan(hri, SRC, DEST);
+    assertEquals(plan.hashCode(), new RegionPlan(hri, SRC, DEST).hashCode());
+    assertEquals(plan, new RegionPlan(hri, SRC, DEST));
 
     // HRI is used for equality
-    HRegionInfo other = new HRegionInfo(TableName.valueOf("other"));
-    assertNotEquals(plan.hashCode(), new RegionPlan(other, source, dest).hashCode());
-    assertNotEquals(plan, new RegionPlan(other, source, dest));
+    HRegionInfo other =
+        new HRegionInfo(TableName.valueOf(name.getMethodName() + "other"));
+    assertNotEquals(plan.hashCode(), new RegionPlan(other, SRC, DEST).hashCode());
+    assertNotEquals(plan, new RegionPlan(other, SRC, DEST));
   }
 }


Mime
View raw message