You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2017/11/16 21:27:17 UTC
hbase git commit: HBASE-19276 RegionPlan should correctly implement
equals and hashCode
Repository: hbase
Updated Branches:
refs/heads/master d8fb10c83 -> 6f9651b41
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 <ap...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/6f9651b4
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/6f9651b4
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/6f9651b4
Branch: refs/heads/master
Commit: 6f9651b41741ac8c0d3ea0bb48cdeebf2101585e
Parents: d8fb10c
Author: Michael Stack <st...@apache.org>
Authored: Wed Nov 15 23:23:28 2017 -0800
Committer: Michael Stack <st...@apache.org>
Committed: Thu Nov 16 13:27:14 2017 -0800
----------------------------------------------------------------------
.../apache/hadoop/hbase/master/RegionPlan.java | 78 +++++++++++++++++---
.../hadoop/hbase/master/TestRegionPlan.java | 76 ++++++++++++++-----
2 files changed, 124 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/6f9651b4/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 10252df..6c91a52 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 = RegionInfo.COMPARATOR.compare(left.hri, 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/6f9651b4/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 8d20179..3d10c9f 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,12 +17,15 @@
*/
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.client.RegionInfo;
+import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.junit.Rule;
@@ -32,27 +35,64 @@ import org.junit.rules.TestName;
@Category({MasterTests.class, 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(name.getMethodName()));
- 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() {
+ RegionInfo hri = RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName())).build();
+ 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() {
+ RegionInfo hri = RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName())).build();
+ 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() {
+ RegionInfo hri = RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName())).build();
+
+ // 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(name.getMethodName() + "other"));
- assertNotEquals(plan.hashCode(), new RegionPlan(other, source, dest).hashCode());
- assertNotEquals(plan, new RegionPlan(other, source, dest));
+ RegionInfo other =
+ RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName() + "other")).build();
+ assertNotEquals(plan.hashCode(), new RegionPlan(other, SRC, DEST).hashCode());
+ assertNotEquals(plan, new RegionPlan(other, SRC, DEST));
}
}