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 10252df16eb..6c91a52ce38 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 { private ServerName dest; public static class RegionPlanComparator implements Comparator, 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 { /** * 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 { 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 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 8d20179b62f..3d10c9fc84f 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)); + 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)); + } - // Source and destination not used for equality - assertEquals(plan.hashCode(), new RegionPlan(hri, dest, source).hashCode()); - assertEquals(plan, new RegionPlan(hri, dest, source)); + @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)); } }