Skip to content

Commit

Permalink
HBASE-25365 The log in move_servers_rsgroup is incorrect (#2742)
Browse files Browse the repository at this point in the history 
Signed-off-by: stack <stack@apache.or>
  • Loading branch information
@ZhaoBQ
ZhaoBQ committed Dec 16, 2020
1 parent 9bdac6c commit 1bb9b78
Showing 1 changed file with 17 additions and 18 deletions.
35 changes: 17 additions & 18 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
View file
Expand Up @@ -954,27 +954,28 @@ private void addRegion(final LinkedList<RegionInfo> regions, RegionInfo hri) {
* located there.
* @param movedServers the servers that are moved to new group
* @param srcGrpServers all servers in the source group, excluding the movedServers
* @param targetGroup the target group
* @param targetGroupName the target group
* @param sourceGroupName the source group
* @throws IOException if moving the server and tables fail
*/
private void moveServerRegionsFromGroup(Set<Address> movedServers, Set<Address> srcGrpServers,
RSGroupInfo targetGroup) throws IOException {
moveRegionsBetweenGroups(movedServers, srcGrpServers, targetGroup, rs -> getRegions(rs),
info -> {
String targetGroupName, String sourceGroupName) throws IOException {
moveRegionsBetweenGroups(movedServers, srcGrpServers, targetGroupName, sourceGroupName,
rs -> getRegions(rs), info -> {
try {
String groupName = RSGroupUtil.getRSGroupInfo(masterServices, this, info.getTable())
.map(RSGroupInfo::getName).orElse(RSGroupInfo.DEFAULT_GROUP);
return groupName.equals(targetGroup.getName());
return groupName.equals(targetGroupName);
} catch (IOException e) {
LOG.warn("Failed to test group for region {} and target group {}", info,
targetGroup.getName());
targetGroupName);
return false;
}
});
}

private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, Set<Address> newRegionsOwners,
RSGroupInfo targetGrp, Function<T, List<RegionInfo>> getRegionsInfo,
String targetGroupName, String sourceGroupName, Function<T, List<RegionInfo>> getRegionsInfo,
Function<RegionInfo, Boolean> validation) throws IOException {
// Get server names corresponding to given Addresses
List<ServerName> movedServerNames = new ArrayList<>(regionsOwners.size());
Expand All @@ -1001,7 +1002,7 @@ private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, Set<Address> new
for (RegionInfo region : getRegionsInfo.apply((T) owner.getAddress())) {
if (!validation.apply(region)) {
LOG.info("Moving region {}, which do not belong to RSGroup {}",
region.getShortNameToLog(), targetGrp.getName());
region.getShortNameToLog(), targetGroupName);
// Move region back to source RSGroup servers
ServerName dest =
masterServices.getLoadBalancer().randomAssignment(region, srcGrpServerNames);
Expand All @@ -1015,17 +1016,16 @@ private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, Set<Address> new
assignmentFutures.add(Pair.newPair(region, future));
} catch (IOException ioe) {
failedRegions.add(region.getRegionNameAsString());
LOG.debug("Move region {} from group failed, will retry, current retry time is {}",
LOG.debug("Move region {} failed, will retry, current retry time is {}",
region.getShortNameToLog(), retry, ioe);
toThrow = ioe;
}
}
}
}
waitForRegionMovement(assignmentFutures, failedRegions, targetGrp.getName(), retry);
waitForRegionMovement(assignmentFutures, failedRegions, sourceGroupName, retry);
if (failedRegions.isEmpty()) {
LOG.info("All regions from server(s) {} moved to target group {}.", movedServerNames,
targetGrp.getName());
LOG.info("All regions from {} are moved back to {}", movedServerNames, sourceGroupName);
return;
} else {
try {
Expand All @@ -1043,7 +1043,7 @@ private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, Set<Address> new
if (!failedRegions.isEmpty()) {
// print failed moved regions, for later process conveniently
String msg = String
.format("move regions for group %s failed, failed regions: %s", targetGrp.getName(),
.format("move regions for group %s failed, failed regions: %s", sourceGroupName,
failedRegions);
LOG.error(msg);
throw new DoNotRetryIOException(
Expand All @@ -1056,9 +1056,9 @@ private <T> void moveRegionsBetweenGroups(Set<T> regionsOwners, Set<Address> new
* completion even if some region movement fails.
*/
private void waitForRegionMovement(List<Pair<RegionInfo, Future<byte[]>>> regionMoveFutures,
Set<String> failedRegions, String tgtGrpName, int retryCount) {
Set<String> failedRegions, String sourceGroupName, int retryCount) {
LOG.info("Moving {} region(s) to group {}, current retry={}", regionMoveFutures.size(),
tgtGrpName, retryCount);
sourceGroupName, retryCount);
for (Pair<RegionInfo, Future<byte[]>> pair : regionMoveFutures) {
try {
pair.getSecond().get();
Expand All @@ -1073,7 +1073,7 @@ private void waitForRegionMovement(List<Pair<RegionInfo, Future<byte[]>>> region
} catch (Exception e) {
failedRegions.add(pair.getFirst().getRegionNameAsString());
LOG.error("Move region {} to group {} failed, will retry on next attempt",
pair.getFirst().getShortNameToLog(), tgtGrpName, e);
pair.getFirst().getShortNameToLog(), sourceGroupName, e);
}
}
}
Expand Down Expand Up @@ -1225,7 +1225,6 @@ public void moveServers(Set<Address> servers, String targetGroupName) throws IOE
if (StringUtils.isEmpty(targetGroupName)) {
throw new ConstraintException("RSGroup cannot be null.");
}
RSGroupInfo targetGroup = getRSGroupInfo(targetGroupName);

// Hold a lock on the manager instance while moving servers to prevent
// another writer changing our state while we are working.
Expand Down Expand Up @@ -1270,7 +1269,7 @@ public void moveServers(Set<Address> servers, String targetGroupName) throws IOE
// MovedServers may be < passed in 'servers'.
Set<Address> movedServers = moveServers(servers, srcGrp.getName(),
targetGroupName);
moveServerRegionsFromGroup(movedServers, srcGrp.getServers(), targetGroup);
moveServerRegionsFromGroup(movedServers, srcGrp.getServers(), targetGroupName, srcGrp.getName());
LOG.info("Move servers done: {} => {}", srcGrp.getName(), targetGroupName);
}
}
Expand Down

0 comments on commit 1bb9b78

Please sign in to comment.