From c6a65ba63fce85ac7c4b62b96ef2bbe6c35d2f00 Mon Sep 17 00:00:00 2001 From: Vasudevan Date: Tue, 4 Sep 2018 16:38:09 +0530 Subject: [PATCH] HBASE-20741 - Split of a region with replicas creates all daughter regions and its replica in same server (Addendum for duo's comments in RB) --- .../assignment/AssignmentManagerUtil.java | 6 +- .../assignment/TestRegionReplicaSplit.java | 104 +++++++++--------- 2 files changed, 53 insertions(+), 57 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java index 77e3a97b997..7f2d11a82ec 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java @@ -50,6 +50,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetRegionIn */ @InterfaceAudience.Private final class AssignmentManagerUtil { + private static final int DEFAULT_REGION_REPLICA = 1; + private AssignmentManagerUtil() { } @@ -142,8 +144,6 @@ final class AssignmentManagerUtil { List regions, int regionReplication, ServerName targetServer) { // create the assign procs only for the primary region using the targetServer TransitRegionStateProcedure[] primaryRegionProcs = regions.stream() - .flatMap(hri -> IntStream.range(0, 1) // yes, only the primary - .mapToObj(i -> RegionReplicaUtil.getRegionInfoForReplica(hri, i))) .map(env.getAssignmentManager().getRegionStates()::getOrCreateRegionStateNode) .map(regionNode -> { TransitRegionStateProcedure proc = @@ -160,7 +160,7 @@ final class AssignmentManagerUtil { } return proc; }).toArray(TransitRegionStateProcedure[]::new); - if (regionReplication == 1) { + if (regionReplication == DEFAULT_REGION_REPLICA) { // this is the default case return primaryRegionProcs; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionReplicaSplit.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionReplicaSplit.java index 315ef83714b..120f36d6a11 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionReplicaSplit.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionReplicaSplit.java @@ -35,6 +35,8 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.Region; import org.apache.hadoop.hbase.testclassification.LargeTests; @@ -66,24 +68,24 @@ public class TestRegionReplicaSplit { private static final byte[] f = HConstants.CATALOG_FAMILY; @BeforeClass - public static void before() throws Exception { + public static void beforeClass() throws Exception { HTU.getConfiguration().setInt("hbase.master.wait.on.regionservers.mintostart", 3); HTU.startMiniCluster(NB_SERVERS); final TableName tableName = TableName.valueOf(TestRegionReplicaSplit.class.getSimpleName()); // Create table then get the single region for our new table. - createTableDirectlyFromHTD(tableName); + createTable(tableName); } @Rule public TestName name = new TestName(); - private static void createTableDirectlyFromHTD(final TableName tableName) throws IOException { - HTableDescriptor htd = new HTableDescriptor(tableName); - htd.setRegionReplication(3); + private static void createTable(final TableName tableName) throws IOException { + TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableName); + builder.setRegionReplication(3); // create a table with 3 replication - table = HTU.createTable(htd, new byte[][] { f }, getSplits(2), + table = HTU.createTable(builder.build(), new byte[][] { f }, getSplits(2), new Configuration(HTU.getConfiguration())); } @@ -101,63 +103,57 @@ public class TestRegionReplicaSplit { HTU.shutdownMiniCluster(); } - @Test(timeout = 60000) public void testRegionReplicaSplitRegionAssignment() throws Exception { - try { - HTU.loadNumericRows(table, f, 0, 3); - // split the table - List regions = new ArrayList(); + HTU.loadNumericRows(table, f, 0, 3); + // split the table + List regions = new ArrayList(); + for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { + for (Region r : rs.getRegionServer().getRegions(table.getName())) { + System.out.println("the region before split is is " + r.getRegionInfo() + + rs.getRegionServer().getServerName()); + regions.add(r.getRegionInfo()); + } + } + HTU.getAdmin().split(table.getName(), Bytes.toBytes(1)); + int count = 0; + while (true) { for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { for (Region r : rs.getRegionServer().getRegions(table.getName())) { - System.out.println("the region before split is is " + r.getRegionInfo() + count++; + } + } + if (count >= 9) { + break; + } + count = 0; + } + List newRegionLocations = new ArrayList(); + for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { + RegionInfo prevInfo = null; + for (Region r : rs.getRegionServer().getRegions(table.getName())) { + if (!regions.contains(r.getRegionInfo()) + && !RegionReplicaUtil.isDefaultReplica(r.getRegionInfo())) { + LOG.info("The region is " + r.getRegionInfo() + " the location is " + rs.getRegionServer().getServerName()); - regions.add(r.getRegionInfo()); - } - } - HTU.getAdmin().split(table.getName(), Bytes.toBytes(1)); - int count = 0; - while (true) { - for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { - for (Region r : rs.getRegionServer().getRegions(table.getName())) { - count++; - } - } - if (count >= 9) { - break; - } - count = 0; - } - List newRegionLocations = new ArrayList(); - for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { - RegionInfo prevInfo = null; - for (Region r : rs.getRegionServer().getRegions(table.getName())) { - if (!regions.contains(r.getRegionInfo()) - && !RegionReplicaUtil.isDefaultReplica(r.getRegionInfo())) { - LOG.info("The region is " + r.getRegionInfo() + " the location is " - + rs.getRegionServer().getServerName()); + if (!RegionReplicaUtil.isDefaultReplica(r.getRegionInfo()) + && newRegionLocations.contains(rs.getRegionServer().getServerName()) + && prevInfo != null + && Bytes.equals(prevInfo.getStartKey(), r.getRegionInfo().getStartKey()) + && Bytes.equals(prevInfo.getEndKey(), r.getRegionInfo().getEndKey())) { + fail("Splitted regions should not be assigned to same region server"); + } else { + prevInfo = r.getRegionInfo(); if (!RegionReplicaUtil.isDefaultReplica(r.getRegionInfo()) - && newRegionLocations.contains(rs.getRegionServer().getServerName()) - && prevInfo != null - && Bytes.equals(prevInfo.getStartKey(), r.getRegionInfo().getStartKey()) - && Bytes.equals(prevInfo.getEndKey(), r.getRegionInfo().getEndKey())) { - fail("Splitted regions should not be assigned to same region server"); - } else { - prevInfo = r.getRegionInfo(); - if (!RegionReplicaUtil.isDefaultReplica(r.getRegionInfo()) - && !newRegionLocations.contains(rs.getRegionServer().getServerName())) { - newRegionLocations.add(rs.getRegionServer().getServerName()); - } + && !newRegionLocations.contains(rs.getRegionServer().getServerName())) { + newRegionLocations.add(rs.getRegionServer().getServerName()); } } } } - // since we assign the daughter regions in round robin fashion, both the daugther region - // replicas will be assigned to two unique servers. - assertEquals("The new regions should be assigned to 3 unique servers ", 3, - newRegionLocations.size()); - } finally { - HTU.getAdmin().disableTable(table.getName()); - HTU.getAdmin().deleteTable(table.getName()); } + // since we assign the daughter regions in round robin fashion, both the daugther region + // replicas will be assigned to two unique servers. + assertEquals("The new regions should be assigned to 3 unique servers ", 3, + newRegionLocations.size()); } }