anoopsjohn commented on a change in pull request #1901:
URL: https://github.com/apache/hbase/pull/1901#discussion_r440757347



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
##########
@@ -413,7 +429,7 @@ private void loadRegions(List<RegionInfo> regionsToMove)
    * Unload regions from given {@link #hostname} using ack/noAck mode and 
{@link #maxthreads}.In
    * noAck mode we do not make sure that region is successfully online on the 
target region
    * server,hence it is best effort.We do not unload regions to hostnames 
given in
-   * {@link #excludeFile}.
+   * {@link #excludeFile}. We only unload regions to hostnames given in {@link 
#designatedFile}.

Review comment:
       Say it **if** this param is provided. This is anyways an optional stuff

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java
##########
@@ -653,21 +673,39 @@ private void deleteFile(String filename) {
   }
 
   /**
-   * @return List of servers from the exclude file in format 'hostname:port'.
+   * @param filename The file should have 'host:port' per line
+   * @return List of servers from the file in format 'hostname:port'.
    */
-  private List<String> readExcludes(String excludeFile) throws IOException {
-    List<String> excludeServers = new ArrayList<>();
-    if (excludeFile == null) {
-      return excludeServers;
+  private List<String> readServersFromFile(String filename) {
+    List<String> servers = new ArrayList<>();
+    if (filename == null) {
+      return servers;
     } else {
       try {
-        Files.readAllLines(Paths.get(excludeFile)).stream().map(String::trim)
-            .filter(((Predicate<String>) 
String::isEmpty).negate()).map(String::toLowerCase)
-            .forEach(excludeServers::add);
+        Files.readAllLines(Paths.get(filename)).stream().map(String::trim)
+          .filter(((Predicate<String>) 
String::isEmpty).negate()).map(String::toLowerCase)
+          .forEach(servers::add);
       } catch (IOException e) {
         LOG.warn("Exception while reading excludes file, continuing anyways", 
e);
       }
-      return excludeServers;
+      return servers;
+    }
+  }
+
+  private void filterDesignatedServers(List<ServerName> onlineServers) {
+    List<String> designatedServers = readServersFromFile(designatedFile);
+    if (designatedServers == null || designatedServers.isEmpty()) {
+      LOG.warn("Designated server is null, use all online server.");

Review comment:
       Pls make the log more meaningful.. We can say no designated servers 
provided in the file + this.designatedFile




----------------------------------------------------------------
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]


Reply via email to