HBASE-28168 Add option in RegionMover.java to isolate one or more reg… #5470
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| import java.nio.file.Files; | ||
| import java.nio.file.Paths; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.EnumSet; | ||
|
|
@@ -53,13 +54,16 @@ | |
| import org.apache.hadoop.hbase.ClusterMetrics.Option; | ||
| import org.apache.hadoop.hbase.HBaseConfiguration; | ||
| import org.apache.hadoop.hbase.HConstants; | ||
| import org.apache.hadoop.hbase.HRegionLocation; | ||
| import org.apache.hadoop.hbase.MetaTableAccessor; | ||
| import org.apache.hadoop.hbase.ServerName; | ||
| import org.apache.hadoop.hbase.UnknownRegionException; | ||
| import org.apache.hadoop.hbase.client.Admin; | ||
| import org.apache.hadoop.hbase.client.Connection; | ||
| import org.apache.hadoop.hbase.client.ConnectionFactory; | ||
| import org.apache.hadoop.hbase.client.DoNotRetryRegionException; | ||
| import org.apache.hadoop.hbase.client.RegionInfo; | ||
| import org.apache.hadoop.hbase.client.Result; | ||
| import org.apache.hadoop.hbase.master.RackManager; | ||
| import org.apache.hadoop.hbase.master.assignment.AssignmentManager; | ||
| import org.apache.hadoop.hbase.net.Address; | ||
|
|
@@ -96,6 +100,7 @@ public class RegionMover extends AbstractHBaseTool implements Closeable { | |
| private boolean ack = true; | ||
| private int maxthreads = 1; | ||
| private int timeout; | ||
| private List<String> isolateRegionIdArray; | ||
| private String loadUnload; | ||
| private String hostname; | ||
| private String filename; | ||
|
|
@@ -112,6 +117,7 @@ private RegionMover(RegionMoverBuilder builder) throws IOException { | |
| this.excludeFile = builder.excludeFile; | ||
| this.designatedFile = builder.designatedFile; | ||
| this.maxthreads = builder.maxthreads; | ||
| this.isolateRegionIdArray = builder.isolateRegionIdArray; | ||
| this.ack = builder.ack; | ||
| this.port = builder.port; | ||
| this.timeout = builder.timeout; | ||
|
|
@@ -156,6 +162,7 @@ public static class RegionMoverBuilder { | |
| private boolean ack = true; | ||
| private int maxthreads = 1; | ||
| private int timeout = Integer.MAX_VALUE; | ||
| private List<String> isolateRegionIdArray = new ArrayList<>(); | ||
| private String hostname; | ||
| private String filename; | ||
| private String excludeFile = null; | ||
|
|
@@ -216,6 +223,14 @@ public RegionMoverBuilder maxthreads(int threads) { | |
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Set the region ID to isolate on the region server. | ||
| */ | ||
| public RegionMoverBuilder isolateRegionIdArray(List<String> isolateRegionIdArray) { | ||
| this.isolateRegionIdArray = isolateRegionIdArray; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Path of file containing hostnames to be excluded during region movement. Exclude file should | ||
| * have 'host:port' per line. Port is mandatory here as we can have many RS running on a single | ||
|
|
@@ -499,7 +514,85 @@ Collection<ServerName> filterRSGroupServers(RSGroupInfo rsgroup, | |
| private void unloadRegions(ServerName server, List<ServerName> regionServers, | ||
| List<RegionInfo> movedRegions) throws Exception { | ||
| while (true) { | ||
| List<RegionInfo> isolateRegionInfoList = Collections.synchronizedList(new ArrayList<>()); | ||
| RegionInfo isolateRegionInfo = null; | ||
| if (isolateRegionIdArray != null && !isolateRegionIdArray.isEmpty()) { | ||
| // Region will be moved to target region server with Ack mode. | ||
| final ExecutorService isolateRegionPool = Executors.newFixedThreadPool(maxthreads); | ||
| List<Future<Boolean>> isolateRegionTaskList = new ArrayList<>(); | ||
| List<RegionInfo> recentlyIsolatedRegion = Collections.synchronizedList(new ArrayList<>()); | ||
| boolean allRegionOpsSuccessful = true; | ||
| for(String isolateRegionId : isolateRegionIdArray) { | ||
| Result result = MetaTableAccessor.scanByRegionEncodedName(conn, | ||
| isolateRegionId); | ||
| HRegionLocation hRegionLocation = MetaTableAccessor.getRegionLocation(conn, | ||
| result.getRow()); | ||
| if (hRegionLocation != null) { | ||
| isolateRegionInfo = hRegionLocation.getRegion(); | ||
| isolateRegionInfoList.add(isolateRegionInfo); | ||
| } else { | ||
| LOG.error("One of the Region " + isolateRegionId + " doesn't exists/can't fetch from" | ||
| + " meta...Quitting now"); | ||
| // We only move the regions if all the regions were found. | ||
| allRegionOpsSuccessful = false; | ||
| break; | ||
| } | ||
| if (hRegionLocation.getServerName() == server) { | ||
| LOG.info("Region " + isolateRegionId + " already exists on server : " | ||
| + server.getHostname()); | ||
| } else { | ||
| Future<Boolean> isolateRegionTask = isolateRegionPool.submit( | ||
| new MoveWithAck(conn, isolateRegionInfo, hRegionLocation.getServerName(), server, | ||
| recentlyIsolatedRegion)); | ||
| isolateRegionTaskList.add(isolateRegionTask); | ||
| } | ||
| } | ||
|
|
||
| if (!allRegionOpsSuccessful) { | ||
| // Failed to fetch one of the region's RegionInfo, so we exit from here. | ||
| break; | ||
|
Contributor
There was a problem hiding this comment. So if we've succeed to submit a first few before the failing to fetch meta location, we won't wait for the ack on the ones we submitted? This could lead the RS even more overloaded, no?
Contributor
Author
There was a problem hiding this comment.
Tool haven't submitted any region move task yet, it is only collection RegionInfo for possible move tasks. At this point, if flag If
Contributor
There was a problem hiding this comment. Aren't you submitting a task for each region on your thread pool on line #562? For example, if you pass 3 regions, the first two are located in meta, but the third isn't, you will break the while, but two tasks for the located regions would be running.
Contributor
Author
There was a problem hiding this comment. I totally missed that. Will take a look into it. Thanks. |
||
| } else if (!isolateRegionTaskList.isEmpty()) { | ||
|
|
||
| // Now that we have fetched all the region's regionInfo, we can move them. | ||
| waitMoveTasksToFinish(isolateRegionPool, isolateRegionTaskList, | ||
| admin.getConfiguration().getLong(MOVE_WAIT_MAX_KEY, DEFAULT_MOVE_WAIT_MAX)); | ||
|
|
||
| // Get New Location for all the regions in isolateRegionIdArray and | ||
| // check that all of them are on the target RS else exit. | ||
| for(String isolateRegionId : isolateRegionIdArray) { | ||
| Result result = MetaTableAccessor.scanByRegionEncodedName(conn, | ||
| isolateRegionId); | ||
| HRegionLocation hRegionLocation = MetaTableAccessor.getRegionLocation(conn, | ||
| result.getRow()); | ||
| if (!(hRegionLocation != null && hRegionLocation.getServerName().equals(server))) { | ||
| LOG.error("One of the Region " + isolateRegionId + " move failed OR stuck in" | ||
| + " transition...Quitting now"); | ||
| allRegionOpsSuccessful = false; | ||
| break; | ||
| } | ||
| } | ||
| } else { | ||
| LOG.info("All regions already exists on server : " + server.getHostname()); | ||
| } | ||
|
|
||
|
|
||
| if (!allRegionOpsSuccessful) { | ||
| // If all the regions are not online on the target server, | ||
| // we don't put RS in decommission mode and exit from here. | ||
| break; | ||
| } else { | ||
| // Once region has been moved to target RS, Let's put the target RS into decommission mode, | ||
| // so master doesn't assign new region to the target RS while we unload the target RS. | ||
| // Also pass 'offload' flag as false since we don't want master to offload the target RS. | ||
| List<ServerName> listOfServer = new ArrayList<>(); | ||
| listOfServer.add(server); | ||
| LOG.info("Putting server : " + server.getHostname() + " in decommission/draining mode"); | ||
| admin.decommissionRegionServers(listOfServer, false); | ||
| } | ||
| } | ||
| List<RegionInfo> regionsToMove = admin.getRegions(server); | ||
| // Remove all the regions from the online Region list, that we just isolated. | ||
| regionsToMove.removeAll(isolateRegionInfoList); | ||
| regionsToMove.removeAll(movedRegions); | ||
| if (regionsToMove.isEmpty()) { | ||
| LOG.info("No Regions to move....Quitting now"); | ||
|
|
@@ -774,6 +867,13 @@ protected void addOptions() { | |
| this.addRequiredOptWithArg("o", "operation", "Expected: load/unload/unload_from_rack"); | ||
| this.addOptWithArg("m", "maxthreads", | ||
| "Define the maximum number of threads to use to unload and reload the regions"); | ||
| this.addOptWithArg("isolateRegionIds", | ||
| "Comma separated list of Region IDs to isolate on the RegionServer and put " | ||
| + " region server host in draining mode. This option should only be used with '-o unload'" | ||
| + " only. By putting region server in decommission/draining mode, master can't assign any" | ||
|
Contributor
There was a problem hiding this comment. nit: remove extra "only" at the and of the sentence: "This option should only be used with '-o isolate_regions' only" |
||
| + " new region on this server. If one or more regions are not found OR failed to isolate" | ||
| + " successfully, utility will exist without putting RS in draining/decommission mode." | ||
| + " Ex. --isolateRegionIds id1,id2,id3"); | ||
| this.addOptWithArg("x", "excludefile", | ||
| "File with <hostname:port> per line to exclude as unload targets; default excludes only " | ||
| + "target host; useful for rack decommisioning."); | ||
|
|
@@ -795,9 +895,14 @@ protected void addOptions() { | |
| protected void processOptions(CommandLine cmd) { | ||
| String hostname = cmd.getOptionValue("r"); | ||
| rmbuilder = new RegionMoverBuilder(hostname); | ||
| this.loadUnload = cmd.getOptionValue("o").toLowerCase(Locale.ROOT); | ||
| if (cmd.hasOption('m')) { | ||
| rmbuilder.maxthreads(Integer.parseInt(cmd.getOptionValue('m'))); | ||
| } | ||
| if (cmd.hasOption("isolateRegionIds") && this.loadUnload.equals("unload")) { | ||
| rmbuilder.isolateRegionIdArray(Arrays.asList( | ||
| cmd.getOptionValue("isolateRegionIds").split(","))); | ||
| } | ||
| if (cmd.hasOption('n')) { | ||
| rmbuilder.ack(false); | ||
| } | ||
|
|
@@ -813,7 +918,6 @@ protected void processOptions(CommandLine cmd) { | |
| if (cmd.hasOption('t')) { | ||
| rmbuilder.timeout(Integer.parseInt(cmd.getOptionValue('t'))); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we are moving regions to the RS we actually wanted to unload? Sounds contradictory, no? And since we are doing it before we actually do the unloads, it may even make situation worse for the case of overloaded RS.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and Yes, it does sound contradictory But Region isolation means we are isolating one or more regions to a single RS, and if that RS have existing regions, then we would have to unload them.
Region isolation can be done in two ways here.
Unload the target RS, move the regions on the target RS to isolate them and put the target RS in draining/decommission mode.. While we do this, HMaster could move another region on the target RS before target RS is in draining/decommission mode. (We could disable/enable balancer switch in this option but it would be unnecessary in my opinion.)
Move the regions on the target RS, put the target RS in draining/decommission mode and then unload the rest of regions from the RS. This would ensure that while we unload regions from the target RS, HMaster can't move any new Regions on the RS.
With option 2, It seems cleaner and more efficient to me. Granted that target RS could be overloaded for some amount of time but it would be easier and cleaner to ensure that target RS only have the regions intended for isolation.
Hope this make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather do #1 with an extra round of unload to address the case master moved regions there during the isolating phase. It would reduce the risk of problems due to overloaded RS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In approach 1, All the regions would be offloaded first including hotspotting region. This can lead to resource contention on the new region where hotspotting moves. Also hotspotting region could get stuck in region transition if region is holding some read-write lock.