Repository: hbase Updated Branches: refs/heads/master 74beb5a3b -> 7a7e55b60
HBASE-19532 AssignProcedure#COMPARATOR may produce incorrect sort order Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/7a7e55b6 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/7a7e55b6 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/7a7e55b6 Branch: refs/heads/master Commit: 7a7e55b601578f18b4eab0faaae060330c707b44 Parents: 74beb5a Author: tedyu <[email protected]> Authored: Mon Dec 18 18:32:24 2017 -0800 Committer: tedyu <[email protected]> Committed: Mon Dec 18 18:32:24 2017 -0800 ---------------------------------------------------------------------- .../master/assignment/AssignProcedure.java | 4 +- .../master/snapshot/TestAssignProcedure.java | 39 +++++++++++++++----- 2 files changed, 31 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/7a7e55b6/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java index 5555062..770d8a4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java @@ -379,7 +379,7 @@ public class AssignProcedure extends RegionTransitionProcedure { return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); } return -1; - } else if (left.getRegionInfo().isMetaRegion()) { + } else if (right.getRegionInfo().isMetaRegion()) { return +1; } if (left.getRegionInfo().getTable().isSystemTable()) { @@ -387,7 +387,7 @@ public class AssignProcedure extends RegionTransitionProcedure { return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); } return -1; - } else if (left.getRegionInfo().getTable().isSystemTable()) { + } else if (right.getRegionInfo().getTable().isSystemTable()) { return +1; } return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); http://git-wip-us.apache.org/repos/asf/hbase/blob/7a7e55b6/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java index ccf88de..1f93ff1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java @@ -19,8 +19,11 @@ package org.apache.hadoop.hbase.master.snapshot; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.CategoryBasedTimeout; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; @@ -40,6 +43,7 @@ import static junit.framework.TestCase.assertTrue; @Category({RegionServerTests.class, SmallTests.class}) public class TestAssignProcedure { + private static final Log LOG = LogFactory.getLog(TestAssignProcedure.class); @Rule public TestName name = new TestName(); @Rule public final TestRule timeout = CategoryBasedTimeout.builder(). withTimeout(this.getClass()). @@ -64,11 +68,17 @@ public class TestAssignProcedure { @Test public void testComparatorWithMetas() { List<AssignProcedure> procedures = new ArrayList<AssignProcedure>(); + RegionInfo user3 = RegionInfoBuilder.newBuilder(TableName.valueOf("user3")).build(); + procedures.add(new AssignProcedure(user3)); + RegionInfo system = RegionInfoBuilder.newBuilder(TableName.NAMESPACE_TABLE_NAME).build(); + procedures.add(new AssignProcedure(system)); RegionInfo user1 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space1")).build(); + RegionInfo user2 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space2")).build(); procedures.add(new AssignProcedure(user1)); RegionInfo meta2 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). setStartKey(Bytes.toBytes("002")).build(); procedures.add(new AssignProcedure(meta2)); + procedures.add(new AssignProcedure(user2)); RegionInfo meta1 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). setStartKey(Bytes.toBytes("001")).build(); procedures.add(new AssignProcedure(meta1)); @@ -76,15 +86,24 @@ public class TestAssignProcedure { RegionInfo meta0 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). setStartKey(Bytes.toBytes("000")).build(); procedures.add(new AssignProcedure(meta0)); - RegionInfo user2 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space2")).build(); - procedures.add(new AssignProcedure(user2)); - RegionInfo system = RegionInfoBuilder.newBuilder(TableName.NAMESPACE_TABLE_NAME).build(); - procedures.add(new AssignProcedure(system)); - procedures.sort(AssignProcedure.COMPARATOR); - assertTrue(procedures.get(0).getRegionInfo().equals(RegionInfoBuilder.FIRST_META_REGIONINFO)); - assertTrue(procedures.get(1).getRegionInfo().equals(meta0)); - assertTrue(procedures.get(2).getRegionInfo().equals(meta1)); - assertTrue(procedures.get(3).getRegionInfo().equals(meta2)); - assertTrue(procedures.get(4).getRegionInfo().getTable().equals(TableName.NAMESPACE_TABLE_NAME)); + for (int i = 0; i < 10; i++) { + Collections.shuffle(procedures); + procedures.sort(AssignProcedure.COMPARATOR); + try { + assertTrue(procedures.get(0).getRegionInfo().equals(RegionInfoBuilder.FIRST_META_REGIONINFO)); + assertTrue(procedures.get(1).getRegionInfo().equals(meta0)); + assertTrue(procedures.get(2).getRegionInfo().equals(meta1)); + assertTrue(procedures.get(3).getRegionInfo().equals(meta2)); + assertTrue(procedures.get(4).getRegionInfo().getTable().equals(TableName.NAMESPACE_TABLE_NAME)); + assertTrue(procedures.get(5).getRegionInfo().equals(user1)); + assertTrue(procedures.get(6).getRegionInfo().equals(user2)); + assertTrue(procedures.get(7).getRegionInfo().equals(user3)); + } catch (Throwable t) { + for (AssignProcedure proc : procedures) { + LOG.debug(proc); + } + throw t; + } + } } }
