From 43688a3f59a03e605073adfe92b8d4833e6b2c7d Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Mon, 11 Dec 2017 22:19:31 -0800 Subject: [PATCH] HBASE-18946 Stochastic load balancer assigns replica regions to the same RS Changed core of AM#createAssignProcedure so we pass list of Regions to assign to the balancer en masse, in one lump. Let the balancer figure what to do with the fat assign. We get back a Map of servers to regions. We then transform that into an array of AssignProcedures to pass to the Assign executor. We sort the array so that meta and system tables are passed to the executor first (and so replicas are clumped together...). Internally the AM executor may divvy up the work into queues but all will be pre-assigned so we should have good distribution (round-robin) regardless of how the queue is processed. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java Cleanup around forceNewPlan. Was confusing. Added a Comparator to sort AssignProcedures so meta and system tables come ahead of user-space tables. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java Remove the forceNewPlan argument on createAssignProcedure. Didn't make sense given we were creating a new AssignProcedure; the arg had no effect. (createAssignProcedures) Recast to feed all regions to the balancer in bulk and to sort the return so meta and system tables take precedence. --- .../hadoop/hbase/procedure2/ProcedureExecutor.java | 3 + .../hadoop/hbase/master/MasterRpcServices.java | 2 +- .../hbase/master/assignment/AssignProcedure.java | 46 +++++- .../hbase/master/assignment/AssignmentManager.java | 128 ++++++++-------- .../master/assignment/MoveRegionProcedure.java | 4 +- .../assignment/RegionTransitionProcedure.java | 7 +- .../master/procedure/CloneSnapshotProcedure.java | 2 + .../master/procedure/CreateTableProcedure.java | 2 + .../master/procedure/EnableTableProcedure.java | 7 +- .../master/procedure/RecoverMetaProcedure.java | 2 +- .../master/procedure/ServerCrashProcedure.java | 4 +- .../master/procedure/TruncateTableProcedure.java | 2 + .../master/assignment/TestAssignmentManager.java | 24 +-- .../hbase/master/snapshot/TestAssignProcedure.java | 90 +++++++++++ .../TestRegionMergeTransactionOnCluster.java | 2 +- .../TestRegionReplicasWithRestartScenarios.java | 164 +++++++++++++++++++++ .../TestSplitTransactionOnCluster.java | 2 +- 17 files changed, 398 insertions(+), 93 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java index 976ad79c68..ac0487165e 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java @@ -880,6 +880,9 @@ public class ProcedureExecutor { public void submitProcedures(final Procedure[] procs) { Preconditions.checkArgument(lastProcId.get() >= 0); Preconditions.checkArgument(isRunning(), "executor not running"); + if (procs == null || procs.length <= 0) { + return; + } // Prepare procedure for (int i = 0; i < procs.length; ++i) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index ce85b66cb4..10f5299e6d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -513,7 +513,7 @@ public class MasterRpcServices extends RSRpcServices master.cpHost.preAssign(regionInfo); } LOG.info(master.getClientIdAuditPrefix() + " assign " + regionInfo.getRegionNameAsString()); - master.getAssignmentManager().assign(regionInfo, true); + master.getAssignmentManager().assign(regionInfo); if (master.cpHost != null) { master.cpHost.postAssign(regionInfo); } 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 33e04fb9e6..5555062f3b 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 @@ -1,4 +1,4 @@ -/** +/* * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.master.assignment; import java.io.IOException; +import java.util.Comparator; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -73,6 +74,9 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProto public class AssignProcedure extends RegionTransitionProcedure { private static final Log LOG = LogFactory.getLog(AssignProcedure.class); + /** + * Set to true when we need recalibrate -- choose a new target -- because original assign failed. + */ private boolean forceNewPlan = false; /** @@ -84,24 +88,24 @@ public class AssignProcedure extends RegionTransitionProcedure { */ protected volatile ServerName targetServer; + /** + * Comparator that will sort AssignProcedures so meta assigns come first, then system table + * assigns and finally user space assigns. + */ + public static final CompareAssignProcedure COMPARATOR = new CompareAssignProcedure(); + public AssignProcedure() { // Required by the Procedure framework to create the procedure on replay super(); } public AssignProcedure(final RegionInfo regionInfo) { - this(regionInfo, false); - } - - public AssignProcedure(final RegionInfo regionInfo, final boolean forceNewPlan) { super(regionInfo); - this.forceNewPlan = forceNewPlan; this.targetServer = null; } public AssignProcedure(final RegionInfo regionInfo, final ServerName destinationServer) { super(regionInfo); - this.forceNewPlan = false; this.targetServer = destinationServer; } @@ -361,4 +365,32 @@ public class AssignProcedure extends RegionTransitionProcedure { protected ProcedureMetrics getProcedureMetrics(MasterProcedureEnv env) { return env.getAssignmentManager().getAssignmentManagerMetrics().getAssignProcMetrics(); } + + /** + * Sort AssignProcedures such that meta and system assigns come first before user-space assigns. + * Have to do it this way w/ distinct Comparator because Procedure is already Comparable on + * 'Env'(?). + */ + public static class CompareAssignProcedure implements Comparator { + @Override + public int compare(AssignProcedure left, AssignProcedure right) { + if (left.getRegionInfo().isMetaRegion()) { + if (right.getRegionInfo().isMetaRegion()) { + return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); + } + return -1; + } else if (left.getRegionInfo().isMetaRegion()) { + return +1; + } + if (left.getRegionInfo().getTable().isSystemTable()) { + if (right.getRegionInfo().getTable().isSystemTable()) { + return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); + } + return -1; + } else if (left.getRegionInfo().getTable().isSystemTable()) { + return +1; + } + return RegionInfo.COMPARATOR.compare(left.getRegionInfo(), right.getRegionInfo()); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index cebe0b0465..b3a04c56c5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.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 @@ -461,7 +461,7 @@ public class AssignmentManager implements ServerListener { proc = createAssignProcedure(metaRegionInfo, serverName); } else { LOG.debug("Assigning " + metaRegionInfo.getRegionNameAsString()); - proc = createAssignProcedure(metaRegionInfo, false); + proc = createAssignProcedure(metaRegionInfo); } ProcedureSyncWait.submitAndWaitProcedure(master.getMasterProcedureExecutor(), proc); } @@ -523,11 +523,7 @@ public class AssignmentManager implements ServerListener { } public void assign(final RegionInfo regionInfo) throws IOException { - assign(regionInfo, true); - } - - public void assign(final RegionInfo regionInfo, final boolean forceNewPlan) throws IOException { - AssignProcedure proc = createAssignProcedure(regionInfo, forceNewPlan); + AssignProcedure proc = createAssignProcedure(regionInfo); ProcedureSyncWait.submitAndWaitProcedure(master.getMasterProcedureExecutor(), proc); } @@ -602,19 +598,67 @@ public class AssignmentManager implements ServerListener { // RegionTransition procedures helpers // ============================================================================================ - public AssignProcedure[] createAssignProcedures(final Collection regionInfo) { - return createAssignProcedures(regionInfo, false); + /** + * Bulk round-robin assign. Use on table creation. + */ + public AssignProcedure[] createAssignProcedures(final List hris) { + if (hris.isEmpty()) { + return null; + } + try { + // Ask the balancer to assign our regions. Pass the regions en masse. The balancer can do + // a better job if it has all the assignments in the one lump. + Map> assignments = getBalancer().roundRobinAssignment(hris, + this.master.getServerManager().createDestinationServersList(null)); + return createAssignProcedure(assignments, hris.size()); + } catch (HBaseIOException hioe) { + LOG.warn("Failed roundRobinAssignment", hioe); + } + // If an error above, fall-through to this simpler assign. Last resort. + return createAssignProceduresWithoutTarget(hris); } - public AssignProcedure[] createAssignProcedures(final Collection regionInfo, - final boolean forceNewPlan) { - if (regionInfo.isEmpty()) return null; - final AssignProcedure[] procs = new AssignProcedure[regionInfo.size()]; + /** + * Create an array of AssignProcedures w/o specifying a target server. + * If no target server, at assign time, we will try to use the former location of the region + * if one exists. This is how we 'retain' the old location across a server restart. + * Used by {@link ServerCrashProcedure} assigning regions on a server that has crashed (SCP is + * also used across a cluster-restart just-in-case to ensure we do cleanup of any old WALs or + * server processes). + */ + public AssignProcedure[] createAssignProceduresWithoutTarget(final List hris) { int index = 0; - for (RegionInfo hri: regionInfo) { - procs[index++] = createAssignProcedure(hri, forceNewPlan); + AssignProcedure[] procedures = new AssignProcedure[hris.size()]; + for (RegionInfo hri : hris) { + procedures[index++] = createAssignProcedure(hri); } - return procs; + return procedures; + } + + // Make this static for the method below where we use it typing the AssignProcedure array we + // return as result. + private static final AssignProcedure [] ASSIGN_PROCEDURE_ARRAY_TYPE = new AssignProcedure[] {}; + + /** + * @param assignments Map of assignments from which we produce an array of assignments. + * @param size Count of assignments to make (the caller may know the total count) + * @return Assignments made from the passed in assignments + */ + AssignProcedure[] createAssignProcedure(Map> assignments, int size) { + List procedures = + new ArrayList(size > 0? size: 8/*Choose an arbitrary size*/); + for (Map.Entry> e: assignments.entrySet()) { + for (RegionInfo ri: e.getValue()) { + AssignProcedure ap = createAssignProcedure(ri, e.getKey()); + ap.setOwner(getProcedureEnvironment().getRequestUser().getShortName()); + procedures.add(ap); + } + } + if (procedures.size() > 0) { + // Sort the procedures so meta and system regions are first in the returned array. + procedures.sort(AssignProcedure.COMPARATOR); + } + return procedures.toArray(ASSIGN_PROCEDURE_ARRAY_TYPE); } // Needed for the following method so it can type the created Array we return @@ -646,14 +690,6 @@ public class AssignmentManager implements ServerListener { } /** - * Called by things like EnableTableProcedure to get a list of AssignProcedure - * to assign the regions of the table. - */ - public AssignProcedure[] createAssignProcedures(final TableName tableName) { - return createAssignProcedures(regionStates.getRegionsOfTable(tableName)); - } - - /** * Called by things like DisableTableProcedure to get a list of UnassignProcedure * to unassign the regions of the table. */ @@ -661,17 +697,8 @@ public class AssignmentManager implements ServerListener { return createUnassignProcedures(regionStates.getTableRegionStateNodes(tableName)); } - /** - * Called by things like ModifyColumnFamilyProcedure to get a list of MoveRegionProcedure - * to reopen the regions of the table. - */ - public MoveRegionProcedure[] createReopenProcedures(final TableName tableName) { - return createReopenProcedures(regionStates.getRegionsOfTable(tableName)); - } - - public AssignProcedure createAssignProcedure(final RegionInfo regionInfo, - final boolean forceNewPlan) { - AssignProcedure proc = new AssignProcedure(regionInfo, forceNewPlan); + public AssignProcedure createAssignProcedure(final RegionInfo regionInfo) { + AssignProcedure proc = new AssignProcedure(regionInfo); proc.setOwner(getProcedureEnvironment().getRequestUser().getShortName()); return proc; } @@ -683,7 +710,7 @@ public class AssignmentManager implements ServerListener { return proc; } - public UnassignProcedure createUnassignProcedure(final RegionInfo regionInfo, + UnassignProcedure createUnassignProcedure(final RegionInfo regionInfo, final ServerName destinationServer, final boolean force) { // If destinationServer is null, figure it. ServerName sn = destinationServer != null? destinationServer: @@ -1257,12 +1284,10 @@ public class AssignmentManager implements ServerListener { setFailoverCleanupDone(true); - // assign offline regions + // Assign offline regions st = System.currentTimeMillis(); - for (RegionInfo regionInfo: getOrderedRegions(regionsToAssign)) { - master.getMasterProcedureExecutor().submitProcedure( - createAssignProcedure(regionInfo, false)); - } + master.getMasterProcedureExecutor().submitProcedures(master.getAssignmentManager(). + createAssignProcedures(regionsToAssign)); et = System.currentTimeMillis(); LOG.info("[STEP-3] " + StringUtils.humanTimeDiff(et - st)); @@ -1366,27 +1391,6 @@ public class AssignmentManager implements ServerListener { return new Pair(ritCount, states.size()); } - /** - * Used when assign regions, this method will put system regions in - * front of user regions - * @param regions - * @return A list of regions with system regions at front - */ - public List getOrderedRegions( - final List regions) { - if (regions == null) return Collections.emptyList(); - - List systemList = new ArrayList<>(); - List userList = new ArrayList<>(); - for (RegionInfo hri : regions) { - if (hri.getTable().isSystemTable()) systemList.add(hri); - else userList.add(hri); - } - // Append userList to systemList - systemList.addAll(userList); - return systemList; - } - // ============================================================================================ // TODO: Region State In Transition // ============================================================================================ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java index 4caed2895d..5940f2fe31 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java @@ -1,4 +1,4 @@ -/** +/* * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -71,7 +71,7 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure futureA = submitProcedure(am.createAssignProcedure(hri, false)); + final Future futureA = submitProcedure(am.createAssignProcedure(hri)); // wait first assign waitOnFuture(futureA); @@ -396,7 +396,7 @@ public class TestAssignmentManager { // Second should be a noop. We should recognize region is already OPEN internally // and skip out doing nothing. // wait second assign - final Future futureB = submitProcedure(am.createAssignProcedure(hri, false)); + final Future futureB = submitProcedure(am.createAssignProcedure(hri)); waitOnFuture(futureB); am.getRegionStates().isRegionInState(hri, State.OPEN); // TODO: What else can we do to ensure just a noop. @@ -419,7 +419,7 @@ public class TestAssignmentManager { rsDispatcher.setMockRsExecutor(new GoodRsExecutor()); // assign the region first - waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); + waitOnFuture(submitProcedure(am.createAssignProcedure(hri))); final Future futureA = submitProcedure(am.createUnassignProcedure(hri, null, false)); @@ -488,7 +488,7 @@ public class TestAssignmentManager { private AssignProcedure createAndSubmitAssign(TableName tableName, int regionId) { RegionInfo hri = createRegionInfo(tableName, regionId); - AssignProcedure proc = am.createAssignProcedure(hri, false); + AssignProcedure proc = am.createAssignProcedure(hri); master.getMasterProcedureExecutor().submitProcedure(proc); return proc; } 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 new file mode 100644 index 0000000000..ccf88de69b --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java @@ -0,0 +1,90 @@ +/* + * Copyright The Apache Software Foundation + * + * 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 regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package org.apache.hadoop.hbase.master.snapshot; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.hadoop.hbase.CategoryBasedTimeout; +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.master.assignment.AssignProcedure; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; +import org.junit.rules.TestRule; + +import static junit.framework.TestCase.assertTrue; + + +@Category({RegionServerTests.class, SmallTests.class}) +public class TestAssignProcedure { + @Rule public TestName name = new TestName(); + @Rule public final TestRule timeout = CategoryBasedTimeout.builder(). + withTimeout(this.getClass()). + withLookingForStuckThread(true). + build(); + + @Test + public void testSimpleComparator() { + List procedures = new ArrayList(); + RegionInfo user1 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space1")).build(); + procedures.add(new AssignProcedure(user1)); + RegionInfo user2 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space2")).build(); + procedures.add(new AssignProcedure(RegionInfoBuilder.FIRST_META_REGIONINFO)); + 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).isMeta()); + assertTrue(procedures.get(1).getRegionInfo().getTable().equals(TableName.NAMESPACE_TABLE_NAME)); + } + + @Test + public void testComparatorWithMetas() { + List procedures = new ArrayList(); + RegionInfo user1 = RegionInfoBuilder.newBuilder(TableName.valueOf("user_space1")).build(); + procedures.add(new AssignProcedure(user1)); + RegionInfo meta2 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). + setStartKey(Bytes.toBytes("002")).build(); + procedures.add(new AssignProcedure(meta2)); + RegionInfo meta1 = RegionInfoBuilder.newBuilder(TableName.META_TABLE_NAME). + setStartKey(Bytes.toBytes("001")).build(); + procedures.add(new AssignProcedure(meta1)); + procedures.add(new AssignProcedure(RegionInfoBuilder.FIRST_META_REGIONINFO)); + 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)); + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java index 035fb9e7f0..ede9764fd8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java @@ -153,7 +153,7 @@ public class TestRegionMergeTransactionOnCluster { RegionStates regionStates = am.getRegionStates(); // We should not be able to assign it again - am.assign(hri, true); + am.assign(hri); assertFalse("Merged region can't be assigned", regionStates.isRegionInTransition(hri)); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java new file mode 100644 index 0000000000..1646a66a04 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java @@ -0,0 +1,164 @@ +/* + * 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 + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.CategoryBasedTimeout; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; +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.TableDescriptorBuilder; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.RegionSplitter; +import org.apache.hadoop.hdfs.DFSConfigKeys; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; +import org.junit.rules.TestRule; + +@Category({RegionServerTests.class, MediumTests.class}) +public class TestRegionReplicasWithRestartScenarios { + private static final Log LOG = LogFactory.getLog(TestRegionReplicasWithRestartScenarios.class); + @Rule public TestName name = new TestName(); + @Rule public final TestRule timeout = CategoryBasedTimeout.builder(). + withTimeout(this.getClass()). + withLookingForStuckThread(true). + build(); + + private static final int NB_SERVERS = 3; + private Table table; + + private static final HBaseTestingUtility HTU = new HBaseTestingUtility(); + private static final byte[] f = HConstants.CATALOG_FAMILY; + + @BeforeClass + public static void beforeClass() throws Exception { + // Reduce the hdfs block size and prefetch to trigger the file-link reopen + // when the file is moved to archive (e.g. compaction) + HTU.getConfiguration().setInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, 8192); + HTU.getConfiguration().setInt(DFSConfigKeys.DFS_CLIENT_READ_PREFETCH_SIZE_KEY, 1); + HTU.getConfiguration().setInt(HConstants.HREGION_MEMSTORE_FLUSH_SIZE, 128 * 1024 * 1024); + HTU.getConfiguration().setInt(">hbase.master.wait.on.regionservers.mintostart", 3); + HTU.startMiniCluster(NB_SERVERS); + } + + @Before + public void before() throws IOException { + TableName tableName = TableName.valueOf(this.name.getMethodName()); + // Create table then get the single region for our new table. + this.table = createTableDirectlyFromHTD(tableName); + } + + @After + public void after() throws IOException { + this.table.close(); + } + + private static Table createTableDirectlyFromHTD(final TableName tableName) throws IOException { + TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableName); + builder.setRegionReplication(3); + return HTU.createTable(builder.build(), new byte[][] { f }, getSplits(20), + new Configuration(HTU.getConfiguration())); + } + + private static byte[][] getSplits(int numRegions) { + RegionSplitter.UniformSplit split = new RegionSplitter.UniformSplit(); + split.setFirstRow(Bytes.toBytes(0L)); + split.setLastRow(Bytes.toBytes(Long.MAX_VALUE)); + return split.split(numRegions); + } + + @AfterClass + public static void afterClass() throws Exception { + HRegionServer.TEST_SKIP_REPORTING_TRANSITION = false; + HTU.shutdownMiniCluster(); + } + + private HRegionServer getRS() { + return HTU.getMiniHBaseCluster().getRegionServer(0); + } + + private HRegionServer getSecondaryRS() { + return HTU.getMiniHBaseCluster().getRegionServer(1); + } + + private HRegionServer getTertiaryRS() { + return HTU.getMiniHBaseCluster().getRegionServer(2); + } + + @Test + public void testRegionReplicasCreated() throws Exception { + Collection onlineRegions = getRS().getOnlineRegionsLocalContext(); + boolean res = checkDuplicates(onlineRegions); + assertFalse(res); + Collection onlineRegions2 = getSecondaryRS().getOnlineRegionsLocalContext(); + res = checkDuplicates(onlineRegions2); + assertFalse(res); + Collection onlineRegions3 = getTertiaryRS().getOnlineRegionsLocalContext(); + checkDuplicates(onlineRegions3); + assertFalse(res); + int totalRegions = onlineRegions.size() + onlineRegions2.size() + onlineRegions3.size(); + assertEquals(62, totalRegions); + } + + private boolean checkDuplicates(Collection onlineRegions3) throws Exception { + ArrayList copyOfRegion = new ArrayList(onlineRegions3); + for (Region region : copyOfRegion) { + RegionInfo regionInfo = region.getRegionInfo(); + RegionInfo regionInfoForReplica = + RegionReplicaUtil.getRegionInfoForDefaultReplica(regionInfo); + int i = 0; + for (Region actualRegion : onlineRegions3) { + if (regionInfoForReplica.equals( + RegionReplicaUtil.getRegionInfoForDefaultReplica(actualRegion.getRegionInfo()))) { + i++; + if (i > 1) { + LOG.info("Duplicate found " + actualRegion.getRegionInfo() + " " + + region.getRegionInfo()); + assertTrue(Bytes.equals(region.getRegionInfo().getStartKey(), + actualRegion.getRegionInfo().getStartKey())); + assertTrue(Bytes.equals(region.getRegionInfo().getEndKey(), + actualRegion.getRegionInfo().getEndKey())); + return true; + } + } + } + } + return false; + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index 8924454ee4..92833fde61 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -716,7 +716,7 @@ public class TestSplitTransactionOnCluster { assertTrue(regionStates.isRegionInState(daughters.get(1).getRegionInfo(), State.OPEN)); // We should not be able to assign it again - am.assign(hri, true); + am.assign(hri); assertFalse("Split region can't be assigned", regionStates.isRegionInTransition(hri)); assertTrue(regionStates.isRegionInState(hri, State.SPLIT)); -- 2.11.0 (Apple Git-81)