HBASE-19532 AssignProcedure#COMPARATOR may produce incorrect sort order

This commit is contained in:
tedyu 2017-12-18 18:33:27 -08:00
parent e7808b61be
commit 1fd22a7b0f
2 changed files with 31 additions and 12 deletions

View File

@ -379,7 +379,7 @@ public class AssignProcedure extends RegionTransitionProcedure {
return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo());
} }
return -1; return -1;
} else if (left.getRegionInfo().isMetaRegion()) { } else if (right.getRegionInfo().isMetaRegion()) {
return +1; return +1;
} }
if (left.getRegionInfo().getTable().isSystemTable()) { if (left.getRegionInfo().getTable().isSystemTable()) {
@ -387,7 +387,7 @@ public class AssignProcedure extends RegionTransitionProcedure {
return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo());
} }
return -1; return -1;
} else if (left.getRegionInfo().getTable().isSystemTable()) { } else if (right.getRegionInfo().getTable().isSystemTable()) {
return +1; return +1;
} }
return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo());

View File

@ -19,8 +19,11 @@
package org.apache.hadoop.hbase.master.snapshot; package org.apache.hadoop.hbase.master.snapshot;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.List; 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.CategoryBasedTimeout;
import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfo;
@ -40,6 +43,7 @@ import static junit.framework.TestCase.assertTrue;
@Category({RegionServerTests.class, SmallTests.class}) @Category({RegionServerTests.class, SmallTests.class})
public class TestAssignProcedure { public class TestAssignProcedure {
private static final Log LOG = LogFactory.getLog(TestAssignProcedure.class);
@Rule public TestName name = new TestName(); @Rule public TestName name = new TestName();
@Rule public final TestRule timeout = CategoryBasedTimeout.builder(). @Rule public final TestRule timeout = CategoryBasedTimeout.builder().
withTimeout(this.getClass()). withTimeout(this.getClass()).
@ -64,11 +68,17 @@ public class TestAssignProcedure {
@Test @Test
public void testComparatorWithMetas() { public void testComparatorWithMetas() {
List<AssignProcedure> procedures = new ArrayList<AssignProcedure>(); 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 user1 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space1")).build();
RegionInfo user2 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space2")).build();
procedures.add(new AssignProcedure(user1)); procedures.add(new AssignProcedure(user1));
RegionInfo meta2 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). RegionInfo meta2 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).
setStartKey(Bytes.toBytes("002")).build(); setStartKey(Bytes.toBytes("002")).build();
procedures.add(new AssignProcedure(meta2)); procedures.add(new AssignProcedure(meta2));
procedures.add(new AssignProcedure(user2));
RegionInfo meta1 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). RegionInfo meta1 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).
setStartKey(Bytes.toBytes("001")).build(); setStartKey(Bytes.toBytes("001")).build();
procedures.add(new AssignProcedure(meta1)); procedures.add(new AssignProcedure(meta1));
@ -76,15 +86,24 @@ public class TestAssignProcedure {
RegionInfo meta0 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). RegionInfo meta0 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME).
setStartKey(Bytes.toBytes("000")).build(); setStartKey(Bytes.toBytes("000")).build();
procedures.add(new AssignProcedure(meta0)); procedures.add(new AssignProcedure(meta0));
RegionInfo user2 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space2")).build(); for (int i = 0; i < 10; i++) {
procedures.add(new AssignProcedure(user2)); Collections.shuffle(procedures);
RegionInfo system = RegionInfoBuilder.newBuilder(TableName.NAMESPACE_TABLE_NAME).build(); procedures.sort(AssignProcedure.COMPARATOR);
procedures.add(new AssignProcedure(system)); try {
procedures.sort(AssignProcedure.COMPARATOR); assertTrue(procedures.get(0).getRegionInfo().equals(RegionInfoBuilder.FIRST_META_REGIONINFO));
assertTrue(procedures.get(0).getRegionInfo().equals(RegionInfoBuilder.FIRST_META_REGIONINFO)); assertTrue(procedures.get(1).getRegionInfo().equals(meta0));
assertTrue(procedures.get(1).getRegionInfo().equals(meta0)); assertTrue(procedures.get(2).getRegionInfo().equals(meta1));
assertTrue(procedures.get(2).getRegionInfo().equals(meta1)); assertTrue(procedures.get(3).getRegionInfo().equals(meta2));
assertTrue(procedures.get(3).getRegionInfo().equals(meta2)); assertTrue(procedures.get(4).getRegionInfo().getTable().equals(TableName.NAMESPACE_TABLE_NAME));
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;
}
}
} }
} }