From 4d24bceb8ab0a8ac94da9dd8c568ec60b0738962 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Wed, 13 Dec 2017 12:38:27 -0800 Subject: [PATCH] Squashed commit of the following: commit 22a2f62696830b23f2db4ed6f8de968aebcb919a Author: Michael Stack Date: Wed Dec 13 12:36:54 2017 -0800 HBASE-19501 [AMv2] Retain assignment across restarts commit 367d8a1884d82eaff354aab15d56baab3f2d1954 Author: Michael Stack Date: Mon Dec 11 22:19:31 2017 -0800 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 + .../org/apache/hadoop/hbase/master/HMaster.java | 8 - .../hadoop/hbase/master/MasterRpcServices.java | 2 +- .../apache/hadoop/hbase/master/ServerManager.java | 33 ++-- .../hbase/master/assignment/AssignProcedure.java | 46 ++++- .../hbase/master/assignment/AssignmentManager.java | 200 +++++++++++++-------- .../master/assignment/MoveRegionProcedure.java | 4 +- .../assignment/RegionTransitionProcedure.java | 7 +- .../master/procedure/RecoverMetaProcedure.java | 2 +- .../master/procedure/ServerCrashProcedure.java | 3 +- .../hadoop/hbase/regionserver/HRegionServer.java | 7 +- .../apache/hadoop/hbase/HBaseTestingUtility.java | 21 ++- .../org/apache/hadoop/hbase/MiniHBaseCluster.java | 20 ++- .../TestMasterOperationsForRegionReplicas.java | 35 ++-- .../master/assignment/TestAssignmentManager.java | 24 +-- .../hbase/master/snapshot/TestAssignProcedure.java | 90 ++++++++++ .../TestRegionMergeTransactionOnCluster.java | 2 +- .../TestRegionReplicasWithRestartScenarios.java | 164 +++++++++++++++++ .../TestSplitTransactionOnCluster.java | 2 +- 19 files changed, 523 insertions(+), 150 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/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 1c576209e8..67e4fdf591 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -761,8 +761,6 @@ public class HMaster extends HRegionServer implements MasterServices { /* * We are active master now... go initialize components we need to run. - * Note, there may be dross in zk from previous runs; it'll get addressed - * below after we determine if cluster startup or failover. */ status.setStatus("Initializing Master file system"); @@ -1173,12 +1171,6 @@ public class HMaster extends HRegionServer implements MasterServices { super.stopServiceThreads(); stopChores(); - // Wait for all the remaining region servers to report in IFF we were - // running a cluster shutdown AND we were NOT aborting. - if (!isAborted() && this.serverManager != null && - this.serverManager.isClusterShutdown()) { - this.serverManager.letRegionServersShutdown(); - } if (LOG.isDebugEnabled()) { LOG.debug("Stopping service threads"); } 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/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index 79ffc8a582..d5ef94de30 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -321,14 +321,14 @@ public class ServerManager { * @param sl the server load on the server * @return true if the server is recorded, otherwise, false */ - boolean checkAndRecordNewServer( - final ServerName serverName, final ServerLoad sl) { - ServerName existingServer = null; + boolean checkAndRecordNewServer(final ServerName serverName, final ServerLoad sl) { + ServerName newerEquivalentServer = null; synchronized (this.onlineServers) { - existingServer = findServerWithSameHostnamePortWithLock(serverName); - if (existingServer != null && (existingServer.getStartcode() > serverName.getStartcode())) { - LOG.info("Server serverName=" + serverName + " rejected; we already have " - + existingServer.toString() + " registered with same hostname and port"); + newerEquivalentServer = getNewerEquivalentServer(serverName); + if (newerEquivalentServer != null) { + LOG.info("ServerName=" + serverName + " rejected; we already have " + + newerEquivalentServer.toString() + " registered with same hostname and port and larger" + + "startcode"); return false; } recordNewServerWithLock(serverName, sl); @@ -343,15 +343,27 @@ public class ServerManager { // Note that we assume that same ts means same server, and don't expire in that case. // TODO: ts can theoretically collide due to clock shifts, so this is a bit hacky. - if (existingServer != null && (existingServer.getStartcode() < serverName.getStartcode())) { + if (newerEquivalentServer != null && + (newerEquivalentServer.getStartcode() < serverName.getStartcode())) { LOG.info("Triggering server recovery; existingServer " + - existingServer + " looks stale, new server:" + serverName); - expireServer(existingServer); + newerEquivalentServer + " looks stale, new server:" + serverName); + expireServer(newerEquivalentServer); } return true; } /** + * @return Name of the new instance of the oldServerName else null. Returned + * ServerName has same hostname and port and a newer startcode. + */ + public ServerName getNewerEquivalentServer(final ServerName oldServerName) { + synchronized (this.onlineServers) { + ServerName result = findServerWithSameHostnamePortWithLock(oldServerName); + return result != null && (result.getStartcode() > oldServerName.getStartcode())? result: null; + } + } + + /** * Checks if the clock skew between the server and the master. If the clock skew exceeds the * configured max, it will throw an exception; if it exceeds the configured warning threshold, * it will log a warning but start normally. @@ -951,7 +963,6 @@ public class ServerManager { String statusStr = "Cluster shutdown requested of master=" + this.master.getServerName(); LOG.info(statusStr); this.clusterShutdown.set(true); - this.master.stop(statusStr); } public boolean isClusterShutdown() { 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..c237058c78 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 @@ -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,23 +598,98 @@ public class AssignmentManager implements ServerListener { // RegionTransition procedures helpers // ============================================================================================ - public AssignProcedure[] createAssignProcedures(final Collection regionInfo) { - return createAssignProcedures(regionInfo, false); + /** + * For {@link ServerCrashProcedure} putting regions back on line on recovery; attempts to + * retain locality by putting regions back up on the new instance of oldLocation, + * the server w/ same host and port but later startcode. + * @return AssignProcedures made out of the passed in hris and passed in + * new instance of oldLocation server if there is one, else we call-through to + * {@link #createAssignProcedures(List)}}. + */ + public AssignProcedure[] createAssignProcedures(final List hris, + ServerName oldLocation) { + if (hris == null || hris.size() <= 0) { + return EMPTY_ASSIGN_PROCEDURE_ARRAY; + } + ServerName newerEquivalentServer = + this.master.getServerManager().getNewerEquivalentServer(oldLocation); + if (newerEquivalentServer != null) { + // Assign all these regions to the new instance of the server so we retain locality; so + // we put the regions back on the server with same port and hostname as oldLocation. + // TODO: Add a stickyness facility to LoadBalancer where you pass in old location and + // balancer tries to keep assignments on new version of old location. Doesn't currently + // exist in balancer. + Map> map = new HashMap<>(); + map.put(newerEquivalentServer, hris); + return createAssignProcedures(map, hris.size()); + } else { + // Fall back to default roundrobin. + return createAssignProcedures(hris); + } } - public AssignProcedure[] createAssignProcedures(final Collection regionInfo, - final boolean forceNewPlan) { - if (regionInfo.isEmpty()) return null; - final AssignProcedure[] procs = new AssignProcedure[regionInfo.size()]; + /** + * @return AssignProcedures made out of the passed in hris and a call + * to the balancer to populate the assigns with targets chosen using round-robin (default + * balancer scheme). If at assign-time, the target chosen is no longer up, thats fine, + * the AssignProcedure will as the balancer for a new target, and so on. + */ + 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 one lump. + Map> assignments = getBalancer().roundRobinAssignment(hris, + this.master.getServerManager().createDestinationServersList(null)); + // Return mid-method! + return createAssignProcedures(assignments, hris.size()); + } catch (HBaseIOException hioe) { + LOG.warn("Failed roundRobinAssignment with " + hris.size() + " regions", hioe); + } + // Fall through to here if the above 'bulk' balancer call failed. Last resort assign. 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 [] EMPTY_ASSIGN_PROCEDURE_ARRAY = new AssignProcedure[] {}; + private static final AssignProcedure [] ASSIGN_PROCEDURE_ARRAY_TYPE = + EMPTY_ASSIGN_PROCEDURE_ARRAY; + + + /** + * @param assignments Map of assignments from which we produce an array of AssignProcedures. + * @param sz Count of assignments to make (the caller may know the total count) + * @return Assignments made from the passed in assignments + */ + AssignProcedure[] createAssignProcedures(Map> assignments, int sz) { + if (assignments == null || assignments.size() <= 0) { + return EMPTY_ASSIGN_PROCEDURE_ARRAY; + } + List procedures = new ArrayList<>(sz > 0? sz: 8/*Arbitrary*/); + 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 - private static final UnassignProcedure [] UNASSIGNED_PROCEDURE_FOR_TYPE_INFO = + private static final UnassignProcedure [] UNASSIGN_PROCEDURE_ARRAY_TYPE = new UnassignProcedure[0]; UnassignProcedure[] createUnassignProcedures(final Collection nodes) { @@ -631,7 +702,7 @@ public class AssignmentManager implements ServerListener { assert node.getRegionLocation() != null: node.toString(); procs.add(createUnassignProcedure(node.getRegionInfo(), node.getRegionLocation(), false)); } - return procs.toArray(UNASSIGNED_PROCEDURE_FOR_TYPE_INFO); + return procs.toArray(UNASSIGN_PROCEDURE_ARRAY_TYPE); } public MoveRegionProcedure[] createReopenProcedures(final Collection regionInfo) { @@ -669,9 +740,8 @@ public class AssignmentManager implements ServerListener { 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; } @@ -1153,20 +1223,16 @@ public class AssignmentManager implements ServerListener { // ============================================================================================ public void joinCluster() throws IOException { final long startTime = System.currentTimeMillis(); - - LOG.info("Joining the cluster..."); - + LOG.info("Joining cluster..."); // Scan hbase:meta to build list of existing regions, servers, and assignment loadMeta(); - for (int i = 0; master.getServerManager().countOfRegionServers() < 1; ++i) { - LOG.info("waiting for RS to join"); + LOG.info("Waiting for RegionServers to join; current count=" + + master.getServerManager().countOfRegionServers()); Threads.sleep(250); } - LOG.info("RS joined. Num RS = " + master.getServerManager().countOfRegionServers()); + LOG.info("Number of RegionServers=" + master.getServerManager().countOfRegionServers()); - // This method will assign all user regions if a clean server startup or - // it will reconstruct master state and cleanup any leftovers from previous master process. boolean failover = processofflineServersWithOnlineRegions(); // Start the RIT chore @@ -1217,55 +1283,60 @@ public class AssignmentManager implements ServerListener { wakeMetaLoadedEvent(); } + /** + * Look at what is in meta and the list of servers that have checked in and make reconciliation. + * @return True if for sure this is a failover where a Master is starting up into an already + * running cluster. + */ // TODO: the assumption here is that if RSs are crashing while we are executing this // they will be handled by the SSH that are put in the ServerManager "queue". // we can integrate this a bit better. private boolean processofflineServersWithOnlineRegions() { - boolean failover = !master.getServerManager().getDeadServers().isEmpty(); - - final Set offlineServersWithOnlineRegions = new HashSet(); - final ArrayList regionsToAssign = new ArrayList(); - long st, et; - - st = System.currentTimeMillis(); + boolean deadServers = !master.getServerManager().getDeadServers().isEmpty(); + final Set offlineServersWithOnlineRegions = new HashSet<>(); + int size = regionStates.getRegionStateNodes().size(); + final List offlineRegionsToAssign = new ArrayList<>(size); + long startTime = System.currentTimeMillis(); + // If deadservers then its a failover, else, we are not sure yet. + boolean failover = deadServers; for (RegionStateNode regionNode: regionStates.getRegionStateNodes()) { + // Region State can be OPEN even if we did controlled cluster shutdown; Master does not close + // the regions in this case. The RegionServer does the close so hbase:meta is state in + // hbase:meta is not update -- Master does this -- and is left with OPEN as region state in + // meta. How to tell difference between ordered shutdown and crashed-down cluster then? We + // can't. Not currently. Perhaps if we updated hbase:meta with CLOSED on ordered shutdown. + // This would slow shutdown though and not all edits would make it in anyways. TODO: Examine. + // Because we can't be sure it an ordered shutdown, we run ServerCrashProcedure always. + // A new facility added here is that ServerCrashProcedure, when it goes to assign the regions, + // will try and put regions back on the new instance of the old server location. This way we + // 'retain' locality, even if an ordered shutdown (Previous SCP did round-robin). if (regionNode.getState() == State.OPEN) { final ServerName serverName = regionNode.getRegionLocation(); if (!master.getServerManager().isServerOnline(serverName)) { offlineServersWithOnlineRegions.add(serverName); + } else { + // Server is online. This a failover. Master is starting into already-running cluster. + failover = true; } } else if (regionNode.getState() == State.OFFLINE) { if (isTableEnabled(regionNode.getTable())) { - regionsToAssign.add(regionNode.getRegionInfo()); + offlineRegionsToAssign.add(regionNode.getRegionInfo()); } } } - et = System.currentTimeMillis(); - LOG.info("[STEP-1] " + StringUtils.humanTimeDiff(et - st)); - - // kill servers with online regions - st = System.currentTimeMillis(); + // Kill servers with online regions just-in-case. Runs ServerCrashProcedure. for (ServerName serverName: offlineServersWithOnlineRegions) { if (!master.getServerManager().isServerOnline(serverName)) { - LOG.info("KILL RS hosting regions but not online " + serverName + - " (master=" + master.getServerName() + ")"); killRegionServer(serverName); } } - et = System.currentTimeMillis(); - LOG.info("[STEP-2] " + StringUtils.humanTimeDiff(et - st)); - setFailoverCleanupDone(true); - // assign offline regions - st = System.currentTimeMillis(); - for (RegionInfo regionInfo: getOrderedRegions(regionsToAssign)) { - master.getMasterProcedureExecutor().submitProcedure( - createAssignProcedure(regionInfo, false)); + // Assign offline regions. Uses round-robin. + if (offlineRegionsToAssign.size() > 0) { + master.getMasterProcedureExecutor().submitProcedures(master.getAssignmentManager(). + createAssignProcedures(offlineRegionsToAssign)); } - et = System.currentTimeMillis(); - LOG.info("[STEP-3] " + StringUtils.humanTimeDiff(et - st)); - return failover; } @@ -1366,27 +1437,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 rsPorts) throws IOException, InterruptedException { + return startMiniHBaseCluster(numMasters, numSlaves, rsPorts, null, null, false, false); } /** * Starts up mini hbase cluster. Usually used after call to * {@link #startMiniCluster(int, int)} when doing stepped startup of clusters. * Usually you won't want this. You'll usually want {@link #startMiniCluster()}. - * @param numMasters - * @param numSlaves + * @param rsPorts Ports that RegionServer should use; pass ports if you want to test cluster + * restart where for sure the regionservers come up on same address+port (but + * just with different startcode); by default mini hbase clusters choose new + * arbitrary ports on each cluster start. * @param create Whether to create a * root or data directory path or not; will overwrite if exists already. * @return Reference to the hbase mini hbase cluster. @@ -990,7 +997,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { * @see {@link #startMiniCluster()} */ public MiniHBaseCluster startMiniHBaseCluster(final int numMasters, - final int numSlaves, Class masterClass, + final int numSlaves, List rsPorts, Class masterClass, Class regionserverClass, boolean create, boolean withWALDir) throws IOException, InterruptedException { @@ -1015,7 +1022,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { Configuration c = new Configuration(this.conf); TraceUtil.initTracer(c); this.hbaseCluster = - new MiniHBaseCluster(c, numMasters, numSlaves, masterClass, regionserverClass); + new MiniHBaseCluster(c, numMasters, numSlaves, rsPorts, masterClass, regionserverClass); // Don't leave here till we've done a successful scan of the hbase:meta Table t = getConnection().getTable(TableName.META_TABLE_NAME); ResultScanner s = t.getScanner(new Scan()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java index e02347d3c3..3f85181308 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java @@ -77,10 +77,19 @@ public class MiniHBaseCluster extends HBaseCluster { */ public MiniHBaseCluster(Configuration conf, int numMasters, int numRegionServers) throws IOException, InterruptedException { - this(conf, numMasters, numRegionServers, null, null); + this(conf, numMasters, numRegionServers, null, null, null); } + /** + * @param rsPorts Ports that RegionServer should use; pass ports if you want to test cluster + * restart where for sure the regionservers come up on same address+port (but + * just with different startcode); by default mini hbase clusters choose new + * arbitrary ports on each cluster start. + * @throws IOException + * @throws InterruptedException + */ public MiniHBaseCluster(Configuration conf, int numMasters, int numRegionServers, + List rsPorts, Class masterClass, Class regionserverClass) throws IOException, InterruptedException { @@ -93,7 +102,7 @@ public class MiniHBaseCluster extends HBaseCluster { // Hadoop 2 CompatibilityFactory.getInstance(MetricsAssertHelper.class).init(); - init(numMasters, numRegionServers, masterClass, regionserverClass); + init(numMasters, numRegionServers, rsPorts, masterClass, regionserverClass); this.initialClusterStatus = getClusterStatus(); } @@ -207,7 +216,7 @@ public class MiniHBaseCluster extends HBaseCluster { } } - private void init(final int nMasterNodes, final int nRegionNodes, + private void init(final int nMasterNodes, final int nRegionNodes, List rsPorts, Class masterClass, Class regionserverClass) throws IOException, InterruptedException { @@ -224,8 +233,11 @@ public class MiniHBaseCluster extends HBaseCluster { masterClass, regionserverClass); // manually add the regionservers as other users - for (int i=0; i rsports = new ArrayList<>(); + for (JVMClusterUtil.RegionServerThread rst: + TEST_UTIL.getHBaseCluster().getLiveRegionServerThreads()) { + rsports.add(rst.getRegionServer().getRpcServer().getListenerAddress().getPort()); + } TEST_UTIL.shutdownMiniHBaseCluster(); - TEST_UTIL.startMiniHBaseCluster(1, numSlaves); + TEST_UTIL.startMiniHBaseCluster(1, numSlaves, rsports); TEST_UTIL.waitTableEnabled(tableName); validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, numReplica, ADMIN.getConnection()); @@ -203,10 +215,10 @@ public class TestMasterOperationsForRegionReplicas { ADMIN.enableTable(tableName); LOG.info(ADMIN.getTableDescriptor(tableName).toString()); assert(ADMIN.isTableEnabled(tableName)); - List regions = TEST_UTIL.getMiniHBaseCluster().getMaster() - .getAssignmentManager().getRegionStates().getRegionsOfTable(tableName); - assertTrue("regions.size=" + regions.size() + ", numRegions=" + numRegions + ", numReplica=" + numReplica, - regions.size() == numRegions * (numReplica + 1)); + List regions = TEST_UTIL.getMiniHBaseCluster().getMaster(). + getAssignmentManager().getRegionStates().getRegionsOfTable(tableName); + assertTrue("regions.size=" + regions.size() + ", numRegions=" + numRegions + + ", numReplica=" + numReplica, regions.size() == numRegions * (numReplica + 1)); //decrease the replica(earlier, table was modified to have a replica count of numReplica + 1) ADMIN.disableTable(tableName); @@ -233,7 +245,6 @@ public class TestMasterOperationsForRegionReplicas { assert(defaultReplicas.size() == numRegions); Collection counts = new HashSet<>(defaultReplicas.values()); assert(counts.size() == 1 && counts.contains(new Integer(numReplica))); - */ } finally { ADMIN.disableTable(tableName); ADMIN.deleteTable(tableName); @@ -342,14 +353,14 @@ public class TestMasterOperationsForRegionReplicas { connection); snapshot.initialize(); Map regionToServerMap = snapshot.getRegionToRegionServerMap(); - assertEquals(regionToServerMap.size(), numRegions * numReplica + 1); //'1' for the namespace + assertEquals(regionToServerMap.size(), numRegions * numReplica + 1); Map> serverToRegionMap = snapshot.getRegionServerToRegionMap(); - assertEquals(serverToRegionMap.keySet().size(), 2); // 1 rs + 1 master + assertEquals("One Region Only", 1, serverToRegionMap.keySet().size()); for (Map.Entry> entry : serverToRegionMap.entrySet()) { if (entry.getKey().equals(TEST_UTIL.getHBaseCluster().getMaster().getServerName())) { continue; } - assertEquals(entry.getValue().size(), numRegions * numReplica); + assertEquals(entry.getValue().size(), numRegions * numReplica + 1); } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java index 1912d1168d..6b9d8e2869 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.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 @@ -216,7 +216,7 @@ public class TestAssignmentManager { final TableName tableName = TableName.valueOf("testAssignAndCrashBeforeResponse"); final RegionInfo hri = createRegionInfo(tableName, 1); rsDispatcher.setMockRsExecutor(new HangThenRSCrashExecutor()); - AssignProcedure proc = am.createAssignProcedure(hri, false); + AssignProcedure proc = am.createAssignProcedure(hri); waitOnFuture(submitProcedure(proc)); } @@ -226,7 +226,7 @@ public class TestAssignmentManager { final RegionInfo hri = createRegionInfo(tableName, 1); rsDispatcher.setMockRsExecutor(new HangOnCloseThenRSCrashExecutor()); for (int i = 0; i < HangOnCloseThenRSCrashExecutor.TYPES_OF_FAILURE; i++) { - AssignProcedure assign = am.createAssignProcedure(hri, false); + AssignProcedure assign = am.createAssignProcedure(hri); waitOnFuture(submitProcedure(assign)); UnassignProcedure unassign = am.createUnassignProcedure(hri, am.getRegionStates().getRegionServerOfRegion(hri), false); @@ -243,7 +243,7 @@ public class TestAssignmentManager { // Loop a bunch of times so we hit various combos of exceptions. for (int i = 0; i < 10; i++) { LOG.info("" + i); - AssignProcedure proc = am.createAssignProcedure(hri, false); + AssignProcedure proc = am.createAssignProcedure(hri); waitOnFuture(submitProcedure(proc)); } } @@ -257,7 +257,7 @@ public class TestAssignmentManager { collectAssignmentManagerMetrics(); rsDispatcher.setMockRsExecutor(new SocketTimeoutRsExecutor(20, 3)); - waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); + waitOnFuture(submitProcedure(am.createAssignProcedure(hri))); rsDispatcher.setMockRsExecutor(new SocketTimeoutRsExecutor(20, 1)); // exception.expect(ServerCrashException.class); @@ -285,7 +285,7 @@ public class TestAssignmentManager { // Test Assign operation failure rsDispatcher.setMockRsExecutor(executor); try { - waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); + waitOnFuture(submitProcedure(am.createAssignProcedure(hri))); fail("unexpected assign completion"); } catch (RetriesExhaustedException e) { // expected exception @@ -294,7 +294,7 @@ public class TestAssignmentManager { // Assign the region (without problems) rsDispatcher.setMockRsExecutor(new GoodRsExecutor()); - waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); + waitOnFuture(submitProcedure(am.createAssignProcedure(hri))); // TODO: Currently unassign just keeps trying until it sees a server crash. // There is no count on unassign. @@ -345,7 +345,7 @@ public class TestAssignmentManager { // Test Assign operation failure rsDispatcher.setMockRsExecutor(executor); try { - waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); + waitOnFuture(submitProcedure(am.createAssignProcedure(hri))); fail("unexpected assign completion"); } catch (RetriesExhaustedException e) { // expected exception @@ -388,7 +388,7 @@ public class TestAssignmentManager { rsDispatcher.setMockRsExecutor(new GoodRsExecutor()); - final Future 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)