mihir6692 commented on code in PR #5470:
URL: https://github.com/apache/hbase/pull/5470#discussion_r1365735209
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java:
##########
@@ -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,
Review Comment:
> So we are moving regions to the RS we actually wanted to unload? Sounds
contradictory, no?
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.
> And since we are doing it before we actually do the unloads, it may even
make situation worse for the case of overloaded RS.
Region isolation can be done in two ways here.
1. 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.)
2. 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
