HBASE-19845 Fix findbugs and error-prone warnings in hbase-rsgroup (branch-2)

Signed-off-by: tedyu <yuzhihong@gmail.com>

hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml

@@ -35,4 +35,5 @@
   <suppress checks="." files=".*/generated-jamon/.*\.java"/>
   <suppress checks="MagicNumberCheck" files=".*/src/test/.*\.java"/>
   <suppress checks="VisibilityModifier" files=".*/src/test/.*\.java"/>
+  <suppress checks="InterfaceIsTypeCheck" files=".*/src/main/.*\.java"/>
 </suppressions>

hbase-rsgroup/pom.xml

@@ -63,6 +63,13 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <configuration>
+          <failOnViolation>true</failOnViolation>
+        </configuration>
+      </plugin>
     </plugins>
     <pluginManagement>
       <plugins>

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java

@@ -22,8 +22,8 @@ import java.util.List;
 import java.util.Set;
 
 import org.apache.hadoop.hbase.TableName;
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.net.Address;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * Group user API interface used between client and server.
@@ -84,7 +84,7 @@ public interface RSGroupAdmin {
    * @param servers set of servers to move
    * @param tables set of tables to move
    * @param targetGroup the target group name
-   * @throws IOException
+   * @throws IOException if moving the server and tables fail
    */
   void moveServersAndTables(Set<Address> servers, Set<TableName> tables,
                             String targetGroup) throws IOException;

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java

@@ -17,13 +17,14 @@
  */
 package org.apache.hadoop.hbase.rsgroup;
 
+import com.google.protobuf.ServiceException;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
 import org.apache.hadoop.hbase.TableName;
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
@@ -44,9 +45,9 @@ import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RSGroupAdmi
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveRSGroupRequest;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveServersRequest;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupProtos;
+import org.apache.yetus.audience.InterfaceAudience;
 
 import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
-import com.google.protobuf.ServiceException;
 
 /**
  * Client used for managing region server group information.

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java

@@ -43,12 +43,13 @@ import org.apache.hadoop.hbase.master.ServerManager;
 import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
 import org.apache.hadoop.hbase.master.assignment.RegionStates.RegionStateNode;
 import org.apache.hadoop.hbase.net.Address;
-import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
-import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
+import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
+
 /**
  * Service to support Region Server Grouping (HBase-6721).
  */
@@ -116,7 +117,10 @@ public class RSGroupAdminServer implements RSGroupAdmin {
     LinkedList<RegionInfo> regions = new LinkedList<>();
     for (Map.Entry<RegionInfo, ServerName> el :
         master.getAssignmentManager().getRegionStates().getRegionAssignments().entrySet()) {
-      if (el.getValue() == null) continue;
+      if (el.getValue() == null) {
+        continue;
+      }
+
       if (el.getValue().getAddress().equals(server)) {
         addRegion(regions, el.getKey());
       }
@@ -133,17 +137,20 @@ public class RSGroupAdminServer implements RSGroupAdmin {
     // If meta, move it last otherwise other unassigns fail because meta is not
     // online for them to update state in. This is dodgy. Needs to be made more
     // robust. See TODO below.
-    if (hri.isMetaRegion()) regions.addLast(hri);
+    if (hri.isMetaRegion()) {
-    else regions.addFirst(hri);
+      regions.addLast(hri);
+    } else {
+      regions.addFirst(hri);
+    }
   }
 
   /**
    * Check servers and tables.
-   * Fail if nulls or if servers and tables not belong to the same group
+   *
    * @param servers servers to move
    * @param tables tables to move
    * @param targetGroupName target group name
-   * @throws IOException
+   * @throws IOException if nulls or if servers and tables not belong to the same group
    */
   private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
                                      String targetGroupName) throws IOException {
@@ -157,7 +164,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
     }
     RSGroupInfo srcGrp = new RSGroupInfo(tmpSrcGrp);
     if (srcGrp.getName().equals(targetGroupName)) {
-      throw new ConstraintException( "Target RSGroup " + targetGroupName +
+      throw new ConstraintException("Target RSGroup " + targetGroupName +
               " is same as source " + srcGrp.getName() + " RSGroup.");
     }
     // Only move online servers
@@ -181,8 +188,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
       }
     }
 
-    if (srcGrp.getServers().size() <= servers.size()
+    if (srcGrp.getServers().size() <= servers.size() && srcGrp.getTables().size() > tables.size()) {
-            && srcGrp.getTables().size() > tables.size() ) {
       throw new ConstraintException("Cannot leave a RSGroup " + srcGrp.getName() +
               " that contains tables without servers to host them.");
     }
@@ -194,7 +200,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
    * @param servers the servers that will move to new group
    * @param tables these tables will be kept on the servers, others will be moved
    * @param targetGroupName the target group name
-   * @throws IOException
+   * @throws IOException if moving the server and tables fail
    */
   private void moveRegionsFromServers(Set<Address> servers, Set<TableName> tables,
       String targetGroupName) throws IOException {
@@ -244,18 +250,19 @@ public class RSGroupAdminServer implements RSGroupAdmin {
   /**
    * Moves every region of tables which should be kept on the servers,
    * but currently they are located on other servers.
-   * @param servers the regions of these servers will be kept on the servers,
+   * @param servers the regions of these servers will be kept on the servers, others will be moved
-   * others will be moved
    * @param tables the tables that will move to new group
    * @param targetGroupName the target group name
-   * @throws IOException
+   * @throws IOException if moving the region fails
    */
   private void moveRegionsToServers(Set<Address> servers, Set<TableName> tables,
       String targetGroupName) throws IOException {
     for (TableName table: tables) {
       LOG.info("Moving region(s) from " + table + " for table move to " + targetGroupName);
-      for (RegionInfo region : master.getAssignmentManager().getRegionStates().getRegionsOfTable(table)) {
+      for (RegionInfo region : master.getAssignmentManager().getRegionStates()
-        ServerName sn = master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(region);
+          .getRegionsOfTable(table)) {
+        ServerName sn = master.getAssignmentManager().getRegionStates()
+            .getRegionServerOfRegion(region);
         if (!servers.contains(sn.getAddress())) {
           master.getAssignmentManager().move(region);
         }
@@ -294,7 +301,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
             + " does not exist.");
       }
       if (srcGrp.getName().equals(targetGroupName)) {
-        throw new ConstraintException( "Target RSGroup " + targetGroupName +
+        throw new ConstraintException("Target RSGroup " + targetGroupName +
             " is same as source " + srcGrp + " RSGroup.");
       }
       // Only move online servers (when moving from 'default') or servers from other
@@ -482,7 +489,10 @@ public class RSGroupAdminServer implements RSGroupAdmin {
 
     synchronized (balancer) {
       // If balance not true, don't run balancer.
-      if (!((HMaster) master).isBalancerOn()) return false;
+      if (!((HMaster) master).isBalancerOn()) {
+        return false;
+      }
+
       if (master.getMasterCoprocessorHost() != null) {
         master.getMasterCoprocessorHost().preBalanceRSGroup(groupName);
       }
@@ -547,7 +557,7 @@ public class RSGroupAdminServer implements RSGroupAdmin {
   @Override
   public void moveServersAndTables(Set<Address> servers, Set<TableName> tables, String targetGroup)
       throws IOException {
-    if (servers == null || servers.isEmpty() ) {
+    if (servers == null || servers.isEmpty()) {
       throw new ConstraintException("The list of servers to move cannot be null or empty.");
     }
     if (tables == null || tables.isEmpty()) {

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java

@@ -42,16 +42,17 @@ import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.master.RegionPlan;
 import org.apache.hadoop.hbase.master.balancer.StochasticLoadBalancer;
 import org.apache.hadoop.hbase.net.Address;
+import org.apache.hadoop.util.ReflectionUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap;
 import org.apache.hbase.thirdparty.com.google.common.collect.LinkedListMultimap;
 import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap;
 import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
 import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
-import org.apache.hadoop.util.ReflectionUtils;
-import org.apache.yetus.audience.InterfaceAudience;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * GroupBasedLoadBalancer, used when Region Server Grouping is configured (HBase-6721)
@@ -209,7 +210,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
       }
 
       for (RegionInfo region : misplacedRegions) {
-        String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable());;
+        String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable());
         RSGroupInfo info = rsGroupInfoManager.getRSGroup(groupName);
         List<ServerName> candidateList = filterOfflineServers(info, servers);
         ServerName server = this.internalBalancer.randomAssignment(region,
@@ -330,8 +331,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
   }
 
   private ServerName findServerForRegion(
-      Map<ServerName, List<RegionInfo>> existingAssignments, RegionInfo region)
+      Map<ServerName, List<RegionInfo>> existingAssignments, RegionInfo region) {
-  {
     for (Map.Entry<ServerName, List<RegionInfo>> entry : existingAssignments.entrySet()) {
       if (entry.getValue().contains(region)) {
         return entry.getKey();
@@ -398,7 +398,10 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
   }
 
   public boolean isOnline() {
-    if (this.rsGroupInfoManager == null) return false;
+    if (this.rsGroupInfoManager == null) {
+      return false;
+    }
+
     return this.rsGroupInfoManager.isOnline();
   }
 

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManager.java

@@ -25,9 +25,9 @@ import java.util.Set;
 
 import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.yetus.audience.InterfaceAudience;
-import org.apache.hadoop.hbase.net.Address;
 
 /**
  * Interface used to manage RSGroupInfo storage. An implementation

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.hbase.rsgroup;
 
+import com.google.protobuf.ServiceException;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -86,8 +88,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
 import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos;
 
-import com.google.protobuf.ServiceException;
-
 /**
  * This is an implementation of {@link RSGroupInfoManager} which makes
  * use of an HBase table as the persistence store for the group information.
@@ -113,7 +113,7 @@ import com.google.protobuf.ServiceException;
  * no other has access concurrently. Reads must be able to continue concurrently.
  */
 @InterfaceAudience.Private
-class RSGroupInfoManagerImpl implements RSGroupInfoManager {
+final class RSGroupInfoManagerImpl implements RSGroupInfoManager {
   private static final Logger LOG = LoggerFactory.getLogger(RSGroupInfoManagerImpl.class);
 
   /** Table descriptor for <code>hbase:rsgroup</code> catalog table */

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java

@@ -21,16 +21,19 @@ package org.apache.hadoop.hbase.rsgroup;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.net.Address;
-import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupProtos;
 import org.apache.hadoop.hbase.protobuf.generated.TableProtos;
+import org.apache.yetus.audience.InterfaceAudience;
 
 @InterfaceAudience.Private
-class RSGroupProtobufUtil {
+final class RSGroupProtobufUtil {
+  private RSGroupProtobufUtil() {
+  }
+
   static RSGroupInfo toGroupInfo(RSGroupProtos.RSGroupInfo proto) {
     RSGroupInfo RSGroupInfo = new RSGroupInfo(proto.getName());
     for(HBaseProtos.ServerName el: proto.getServersList()) {

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupableBalancer.java

@@ -18,8 +18,8 @@
 
 package org.apache.hadoop.hbase.rsgroup;
 
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.master.LoadBalancer;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * Marker Interface. RSGroups feature will check for a LoadBalancer

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/Utility.java

@@ -23,22 +23,28 @@ import java.util.HashSet;
 import java.util.Set;
 
 import org.apache.hadoop.hbase.ServerName;
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.master.MasterServices;
 import org.apache.hadoop.hbase.net.Address;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * Utility for this RSGroup package in hbase-rsgroup.
  */
 @InterfaceAudience.Private
-class Utility {
+final class Utility {
+  private Utility() {
+  }
+
   /**
-   * @param master
+   * @param master the master to get online servers for
    * @return Set of online Servers named for their hostname and port (not ServerName).
    */
   static Set<Address> getOnlineServers(final MasterServices master) {
     Set<Address> onlineServers = new HashSet<Address>();
-    if (master == null) return onlineServers;
+    if (master == null) {
+      return onlineServers;
+    }
+
     for(ServerName server: master.getServerManager().getOnlineServers().keySet()) {
       onlineServers.add(server.getAddress());
     }

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java

@@ -74,8 +74,7 @@ public class TestRSGroupBasedLoadBalancer {
   private static RSGroupBasedLoadBalancer loadBalancer;
   private static SecureRandom rand;
 
-  static String[]  groups = new String[] { RSGroupInfo.DEFAULT_GROUP, "dg2", "dg3",
+  static String[]  groups = new String[] { RSGroupInfo.DEFAULT_GROUP, "dg2", "dg3", "dg4" };
-      "dg4" };
   static TableName table0 = TableName.valueOf("dt0");
   static TableName[] tables =
       new TableName[] { TableName.valueOf("dt1"),
@@ -111,8 +110,6 @@ public class TestRSGroupBasedLoadBalancer {
    *
    * Invariant is that all servers of the group should be hosting either floor(average) or
    * ceiling(average)
-   *
-   * @throws Exception
    */
   @Test
   public void testBalanceCluster() throws Exception {
@@ -164,12 +161,6 @@ public class TestRSGroupBasedLoadBalancer {
 
   /**
    * All regions have an assignment.
-   *
-   * @param regions
-   * @param servers
-   * @param assignments
-   * @throws java.io.IOException
-   * @throws java.io.FileNotFoundException
    */
   private void assertImmediateAssignment(List<RegionInfo> regions,
                                          List<ServerName> servers,
@@ -194,8 +185,6 @@ public class TestRSGroupBasedLoadBalancer {
    * Round-robin. Should yield a balanced cluster so same invariant as the
    * load balancer holds, all servers holding either floor(avg) or
    * ceiling(avg).
-   *
-   * @throws Exception
    */
   @Test
   public void testBulkAssignment() throws Exception {
@@ -241,10 +230,7 @@ public class TestRSGroupBasedLoadBalancer {
     assertFalse(misplacedRegions.contains(ri));
   }
   /**
-   * Test the cluster startup bulk assignment which attempts to retain
+   * Test the cluster startup bulk assignment which attempts to retain assignment info.
-   * assignment info.
-   *
-   * @throws Exception
    */
   @Test
   public void testRetainAssignment() throws Exception {
@@ -264,8 +250,7 @@ public class TestRSGroupBasedLoadBalancer {
   }
 
   /**
-   * Test BOGUS_SERVER_NAME among groups do not overwrite each other
+   * Test BOGUS_SERVER_NAME among groups do not overwrite each other.
-   * @throws Exception
    */
   @Test
   public void testRoundRobinAssignment() throws Exception {
@@ -303,11 +288,6 @@ public class TestRSGroupBasedLoadBalancer {
    * <li>If a region had an existing assignment to a server with the same
    * address a a currently online server, it will be assigned to it
    * </ul>
-   *
-   * @param existing
-   * @param assignment
-   * @throws java.io.IOException
-   * @throws java.io.FileNotFoundException
    */
   private void assertRetainedAssignment(
       Map<RegionInfo, ServerName> existing, List<ServerName> servers,
@@ -320,8 +300,9 @@ public class TestRSGroupBasedLoadBalancer {
       assertTrue(
           "Region assigned to server that was not listed as online",
           onlineServerSet.contains(a.getKey()));
-      for (RegionInfo r : a.getValue())
+      for (RegionInfo r : a.getValue()) {
         assignedRegions.add(r);
+      }
     }
     assertEquals(existing.size(), assignedRegions.size());
 

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java

@@ -40,8 +40,6 @@ import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.master.ServerManager;
 import org.apache.hadoop.hbase.master.snapshot.SnapshotManager;
 import org.apache.hadoop.hbase.net.Address;
-import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.After;
@@ -53,7 +51,10 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
+import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos;
 
 @Category({MediumTests.class})
 public class TestRSGroups extends TestRSGroupsBase {

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java

@@ -345,8 +345,10 @@ public abstract class TestRSGroupsBase {
       @Override
       public boolean evaluate() throws Exception {
         List<String> regions = getTableRegionMap().get(tableName);
-        if (regions == null)
+        if (regions == null) {
           return false;
+        }
+
         return getTableRegionMap().get(tableName).size() >= 5;
       }
     });
@@ -478,8 +480,9 @@ public abstract class TestRSGroupsBase {
       @Override
       public boolean evaluate() throws Exception {
         List<String> regions = getTableRegionMap().get(tableName);
-        if (regions == null)
+        if (regions == null) {
           return false;
+        }
         return getTableRegionMap().get(tableName).size() >= 6;
       }
     });
@@ -518,8 +521,8 @@ public abstract class TestRSGroupsBase {
     });
 
     // Lets move this region to the new group.
-    TEST_UTIL.getAdmin().move(Bytes.toBytes(RegionInfo.encodeRegionName(Bytes.toBytes(targetRegion))),
+    TEST_UTIL.getAdmin().move(Bytes.toBytes(RegionInfo.encodeRegionName(
-        Bytes.toBytes(targetServer.getServerName()));
+        Bytes.toBytes(targetRegion))), Bytes.toBytes(targetServer.getServerName()));
     TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate<Exception>() {
       @Override
       public boolean evaluate() throws Exception {
@@ -678,11 +681,14 @@ public abstract class TestRSGroupsBase {
       @Override
       public boolean evaluate() throws Exception {
         List<String> regionsA = getTableRegionMap().get(tableNameA);
-        if (regionsA == null)
+        if (regionsA == null) {
           return false;
+        }
+
         List<String> regionsB = getTableRegionMap().get(tableNameB);
-        if (regionsB == null)
+        if (regionsB == null) {
           return false;
+        }
 
         return getTableRegionMap().get(tableNameA).size() >= 1
                 && getTableRegionMap().get(tableNameB).size() >= 1;
@@ -706,7 +712,8 @@ public abstract class TestRSGroupsBase {
             rsGroupAdmin.getRSGroupInfoOfTable(tableNameB).getName());
 
     //verify tables' not exist in old group
-    Set<TableName> DefaultTables = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP).getTables();
+    Set<TableName> DefaultTables = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP)
+        .getTables();
     assertFalse(DefaultTables.contains(tableNameA));
     assertFalse(DefaultTables.contains(tableNameB));
 
@@ -760,8 +767,9 @@ public abstract class TestRSGroupsBase {
       @Override
       public boolean evaluate() throws Exception {
         List<String> regions = getTableRegionMap().get(tableName);
-        if (regions == null)
+        if (regions == null) {
           return false;
+        }
         return getTableRegionMap().get(tableName).size() >= 5;
       }
     });
@@ -850,7 +858,8 @@ public abstract class TestRSGroupsBase {
             rsGroupAdmin.getRSGroupInfoOfTable(tableName).getName());
 
     //verify servers' not exist in old group
-    Set<Address> defaultServers = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP).getServers();
+    Set<Address> defaultServers = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP)
+        .getServers();
     assertFalse(defaultServers.contains(targetServer.getAddress()));
 
     //verify servers' exist in new group
@@ -858,7 +867,8 @@ public abstract class TestRSGroupsBase {
     assertTrue(newGroupServers.contains(targetServer.getAddress()));
 
     //verify tables' not exist in old group
-    Set<TableName> defaultTables = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP).getTables();
+    Set<TableName> defaultTables = rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP)
+        .getTables();
     assertFalse(defaultTables.contains(tableName));
 
     //verify tables' exist in new group

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java

@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.rsgroup;
 import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
@@ -171,6 +172,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
           TEST_TABLE.toString()).size());
     } catch (Throwable e) {
       LOG.error("error during call of AccessControlClient.getUserPermissions. ", e);
+      fail("error during call of AccessControlClient.getUserPermissions.");
     }
   }
 

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdminClient.java

@@ -17,12 +17,14 @@
  */
 package org.apache.hadoop.hbase.rsgroup;
 
-import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
+import java.io.ByteArrayInputStream;
-import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.client.ConnectionFactory;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.Scan;
@@ -32,15 +34,14 @@ import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupProtos;
 import org.apache.hadoop.hbase.zookeeper.ZKUtil;
+import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
 import org.apache.hadoop.hbase.zookeeper.ZNodePaths;
+import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.KeeperException;
 import org.junit.Assert;
 
-import java.io.ByteArrayInputStream;
+import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
-import java.io.IOException;
+import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 @InterfaceAudience.Private
 public class VerifyingRSGroupAdminClient implements RSGroupAdmin {
@@ -51,7 +52,8 @@ public class VerifyingRSGroupAdminClient implements RSGroupAdmin {
   public VerifyingRSGroupAdminClient(RSGroupAdmin RSGroupAdmin, Configuration conf)
       throws IOException {
     wrapped = RSGroupAdmin;
-    table = ConnectionFactory.createConnection(conf).getTable(RSGroupInfoManager.RSGROUP_TABLE_NAME);
+    table = ConnectionFactory.createConnection(conf)
+        .getTable(RSGroupInfoManager.RSGROUP_TABLE_NAME);
     zkw = new ZKWatcher(conf, this.getClass().getSimpleName(), null);
   }
 
@@ -105,7 +107,8 @@ public class VerifyingRSGroupAdminClient implements RSGroupAdmin {
   }
 
   @Override
-  public void moveServersAndTables(Set<Address> servers, Set<TableName> tables, String targetGroup) throws IOException {
+  public void moveServersAndTables(Set<Address> servers, Set<TableName> tables, String targetGroup)
+      throws IOException {
     wrapped.moveServersAndTables(servers, tables, targetGroup);
     verify();
   }