HBASE-19601 Fixed Checkstyle errors in hbase-rsgroup and enabled Chec…

…kstyle to fail on violations
shadow-tx · Dec 24, 2017 · c24cf2d · c24cf2d
1 parent ab6571e
commit c24cf2d
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 97 deletions.
diff --git a/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml b/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="RSGroupableBalancer.java"/>
 </suppressions>
diff --git a/hbase-rsgroup/pom.xml b/hbase-rsgroup/pom.xml
@@ -68,6 +68,22 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <executions>
+          <execution>
+            <id>checkstyle</id>
+            <phase>validate</phase>
+            <goals>
+              <goal>check</goal>
+            </goals>
+            <configuration>
+              <failOnViolation>true</failOnViolation>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
     </plugins>
     <pluginManagement>
       <plugins>

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdmin.java
@@ -22,8 +22,8 @@
 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;

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminClient.java b/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.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.hadoop.hbase.shaded.com.google.common.collect.Sets;
-import com.google.protobuf.ServiceException;
 
 /**
  * Client used for managing region server group information.

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java
@@ -18,6 +18,10 @@
 
 package org.apache.hadoop.hbase.rsgroup;
 
+import com.google.protobuf.RpcCallback;
+import com.google.protobuf.RpcController;
+import com.google.protobuf.Service;
+
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashSet;
@@ -26,10 +30,6 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import com.google.protobuf.RpcCallback;
-import com.google.protobuf.RpcController;
-import com.google.protobuf.Service;
-
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
@@ -74,11 +74,12 @@
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveRSGroupResponse;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveServersRequest;
 import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveServersResponse;
-import org.apache.hadoop.hbase.shaded.com.google.common.collect.Sets;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hadoop.hbase.shaded.com.google.common.collect.Sets;
+
 // TODO: Encapsulate MasterObserver functions into separate subclass.
 @CoreCoprocessor
 @InterfaceAudience.Private
@@ -231,7 +232,8 @@ public void removeRSGroup(RpcController controller,
     public void balanceRSGroup(RpcController controller,
         BalanceRSGroupRequest request, RpcCallback<BalanceRSGroupResponse> done) {
       BalanceRSGroupResponse.Builder builder = BalanceRSGroupResponse.newBuilder();
-      LOG.info(master.getClientIdAuditPrefix() + " balance rsgroup, group=" + request.getRSGroupName());
+      LOG.info(master.getClientIdAuditPrefix() + " balance rsgroup, group=" +
+              request.getRSGroupName());
       try {
         builder.setBalanceRan(groupAdminServer.balanceRSGroup(request.getRSGroupName()));
       } catch (IOException e) {
@@ -263,7 +265,8 @@ public void getRSGroupInfoOfServer(RpcController controller,
       try {
         Address hp = Address.fromParts(request.getServer().getHostName(),
             request.getServer().getPort());
-        LOG.info(master.getClientIdAuditPrefix() + " initiates rsgroup info retrieval, server=" + hp);
+        LOG.info(master.getClientIdAuditPrefix() + " initiates rsgroup info retrieval, server=" +
+                hp);
         RSGroupInfo RSGroupInfo = groupAdminServer.getRSGroupOfServer(hp);
         if (RSGroupInfo != null) {
           builder.setRSGroupInfo(RSGroupProtobufUtil.toProtoGroupInfo(RSGroupInfo));

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
@@ -43,12 +43,13 @@
 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.hadoop.hbase.shaded.com.google.common.collect.Lists;
-import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists;
+import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps;
+
 /**
  * Service to support Region Server Grouping (HBase-6721).
  */
@@ -116,7 +117,10 @@ private List<RegionInfo> getRegions(final Address server) {
     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 @@ private void addRegion(final LinkedList<RegionInfo> regions, RegionInfo hri) {
     // 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);
-    else regions.addFirst(hri);
+    if (hri.isMetaRegion()) {
+      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 @@ private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
     }
     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 @@ private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
       }
     }
 
-    if (srcGrp.getServers().size() <= servers.size()
-            && srcGrp.getTables().size() > tables.size() ) {
+    if (srcGrp.getServers().size() <= servers.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 @@ private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
    * @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 @@ private void moveRegionsFromServers(Set<Address> servers, Set<TableName> tables,
   /**
    * 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,
-   * others will be moved
+   * @param servers the regions of these servers will be kept on the servers, 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)) {
-        ServerName sn = master.getAssignmentManager().getRegionStates().getRegionServerOfRegion(region);
+      for (RegionInfo region : master.getAssignmentManager().getRegionStates()
+              .getRegionsOfTable(table)) {
+        ServerName sn = master.getAssignmentManager().getRegionStates()
+                .getRegionServerOfRegion(region);
         if (!servers.contains(sn.getAddress())) {
           master.getAssignmentManager().move(region);
         }
@@ -294,7 +301,7 @@ public void moveServers(Set<Address> servers, String targetGroupName)
             + " 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 boolean balanceRSGroup(String groupName) throws IOException {
 
     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 RSGroupInfo getRSGroupOfServer(Address hostPort) throws IOException {
   @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()) {

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
@@ -43,16 +43,17 @@
 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.hadoop.hbase.shaded.com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.ArrayListMultimap;
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.LinkedListMultimap;
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.ListMultimap;
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists;
 import org.apache.hadoop.hbase.shaded.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)
@@ -324,8 +325,7 @@ private Set<RegionInfo> getMisplacedRegions(
   }
 
   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();
@@ -392,7 +392,10 @@ public void initialize() throws HBaseIOException {
   }
 
   public boolean isOnline() {
-    if (this.rsGroupInfoManager == null) return false;
+    if (this.rsGroupInfoManager == null) {
+      return false;
+    }
+
     return this.rsGroupInfoManager.isOnline();
   }
 

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManager.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManager.java
@@ -18,16 +18,15 @@
 
 package org.apache.hadoop.hbase.rsgroup;
 
-
 import java.io.IOException;
 import java.util.List;
 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

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java b/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;
@@ -81,14 +83,13 @@
 import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.Lists;
 import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps;
 import org.apache.hadoop.hbase.shaded.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.
@@ -114,7 +115,7 @@
  * 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 */

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupProtobufUtil.java
@@ -21,15 +21,18 @@
 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.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()) {

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupableBalancer.java b/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