virajjasani commented on a change in pull request #1901:
URL: https://github.com/apache/hbase/pull/1901#discussion_r443212331
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
##########
@@ -435,6 +451,10 @@ public boolean unload() throws InterruptedException,
ExecutionException, Timeout
LOG.debug("List of region servers: {}", regionServers);
return false;
}
+ // Remove RS present not in the designated file
+ if (designatedFile != null) {
+ filterDesignatedServers(regionServers);
Review comment:
We can combine `filterDesignatedServers()` and `stripExcludes()`.
If you see carefully, both are doing the same thing except for one is
excluding what is present in file and another is including. So whether to
include/exclude all RS present in given file is something you can provide as
boolean argument and act accordingly.
Something like:
```
private void includeExcludeRegionServers(String fileName, List<ServerName>
regionServers, boolean isInclude)
```
For, filterDesignatedServers(), it can be:
```
includeExcludeRegionServers(designatedFile, onlineServers, true);
```
and stripExcludes can be replaced with:
```
includeExcludeRegionServers(excludeFile, regionServers, false);
```
Sounds good?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
