hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject [14/30] hbase git commit: HBASE-19239 Fix findbugs and error-prone issues
Date Sat, 18 Nov 2017 01:22:42 GMT
HBASE-19239 Fix findbugs and error-prone issues

Fixes for hbase-rsgroup


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b9475d3e
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b9475d3e
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b9475d3e

Branch: refs/heads/branch-1.4
Commit: b9475d3e759564676ba881610396af6a45fc725b
Parents: 108ea30
Author: Andrew Purtell <apurtell@apache.org>
Authored: Wed Nov 15 18:47:45 2017 -0800
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Fri Nov 17 17:12:11 2017 -0800

----------------------------------------------------------------------
 .../hbase/rsgroup/RSGroupAdminServer.java       |  8 ++--
 .../hbase/rsgroup/RSGroupBasedLoadBalancer.java | 45 ++++++++------------
 .../hadoop/hbase/rsgroup/RSGroupInfo.java       |  4 +-
 .../balancer/TestRSGroupBasedLoadBalancer.java  | 27 ------------
 .../hadoop/hbase/rsgroup/TestRSGroupsBase.java  | 36 +++++++++-------
 .../hbase/client/rsgroup/TestShellRSGroups.java |  2 +-
 6 files changed, 44 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
----------------------------------------------------------------------
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
index 125e08e..0003df0 100644
--- 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
@@ -314,15 +314,15 @@ public class RSGroupAdminServer implements RSGroupAdmin {
       if (master.getMasterCoprocessorHost() != null) {
         master.getMasterCoprocessorHost().preRemoveRSGroup(name);
       }
-      RSGroupInfo RSGroupInfo = getRSGroupInfoManager().getRSGroup(name);
-      if(RSGroupInfo == null) {
+      RSGroupInfo groupInfo = getRSGroupInfoManager().getRSGroup(name);
+      if(groupInfo == null) {
         throw new ConstraintException("Group "+name+" does not exist");
       }
-      int tableCount = RSGroupInfo.getTables().size();
+      int tableCount = groupInfo.getTables().size();
       if (tableCount > 0) {
         throw new ConstraintException("Group "+name+" must have no associated tables: "+tableCount);
       }
-      int serverCount = RSGroupInfo.getServers().size();
+      int serverCount = groupInfo.getServers().size();
       if(serverCount > 0) {
         throw new ConstraintException(
             "Group "+name+" must have no associated servers: "+serverCount);

http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
----------------------------------------------------------------------
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
index e1bcb25..049e723 100644
--- 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
@@ -77,7 +77,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer, LoadBalanc
   private Configuration config;
   private ClusterStatus clusterStatus;
   private MasterServices masterServices;
-  private RSGroupInfoManager RSGroupInfoManager;
+  private RSGroupInfoManager infoManager;
   private LoadBalancer internalBalancer;
 
   //used during reflection by LoadBalancerFactory
@@ -88,7 +88,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer, LoadBalanc
   //This constructor should only be used for unit testing
   @InterfaceAudience.Private
   public RSGroupBasedLoadBalancer(RSGroupInfoManager RSGroupInfoManager) {
-    this.RSGroupInfoManager = RSGroupInfoManager;
+    this.infoManager = RSGroupInfoManager;
   }
 
   @Override
@@ -133,7 +133,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       regionPlans.add(new RegionPlan(regionInfo, null, null));
     }
     try {
-      for (RSGroupInfo info : RSGroupInfoManager.listRSGroups()) {
+      for (RSGroupInfo info : infoManager.listRSGroups()) {
         Map<ServerName, List<HRegionInfo>> groupClusterState =
             new HashMap<ServerName, List<HRegionInfo>>();
         for (Address addr : info.getServers()) {
@@ -192,7 +192,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       Set<HRegionInfo> misplacedRegions = getMisplacedRegions(regions);
       for (HRegionInfo region : regions.keySet()) {
         if (!misplacedRegions.contains(region)) {
-          String groupName = RSGroupInfoManager.getRSGroupOfTable(region.getTable());
+          String groupName = infoManager.getRSGroupOfTable(region.getTable());
           groupToRegion.put(groupName, region);
         }
       }
@@ -201,7 +201,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       for (String key : groupToRegion.keySet()) {
         Map<HRegionInfo, ServerName> currentAssignmentMap = new TreeMap<HRegionInfo,
ServerName>();
         List<HRegionInfo> regionList = groupToRegion.get(key);
-        RSGroupInfo info = RSGroupInfoManager.getRSGroup(key);
+        RSGroupInfo info = infoManager.getRSGroup(key);
         List<ServerName> candidateList = filterOfflineServers(info, servers);
         for (HRegionInfo region : regionList) {
           currentAssignmentMap.put(region, regions.get(region));
@@ -213,9 +213,9 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       }
 
       for (HRegionInfo region : misplacedRegions) {
-        String groupName = RSGroupInfoManager.getRSGroupOfTable(
+        String groupName = infoManager.getRSGroupOfTable(
             region.getTable());
-        RSGroupInfo info = RSGroupInfoManager.getRSGroup(groupName);
+        RSGroupInfo info = infoManager.getRSGroup(groupName);
         List<ServerName> candidateList = filterOfflineServers(info, servers);
         ServerName server = this.internalBalancer.randomAssignment(region,
             candidateList);
@@ -261,14 +261,14 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
     ListMultimap<String, ServerName> serverMap) throws HBaseIOException {
     try {
       for (HRegionInfo region : regions) {
-        String groupName = RSGroupInfoManager.getRSGroupOfTable(region.getTable());
+        String groupName = infoManager.getRSGroupOfTable(region.getTable());
         if(groupName == null) {
           LOG.warn("Group for table "+region.getTable()+" is null");
         }
         regionMap.put(groupName, region);
       }
       for (String groupKey : regionMap.keySet()) {
-        RSGroupInfo info = RSGroupInfoManager.getRSGroup(groupKey);
+        RSGroupInfo info = infoManager.getRSGroup(groupKey);
         serverMap.putAll(groupKey, filterOfflineServers(info, servers));
         if(serverMap.get(groupKey).size() < 1) {
           serverMap.put(groupKey, LoadBalancer.BOGUS_SERVER_NAME);
@@ -285,7 +285,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       return filterServers(RSGroupInfo.getServers(), onlineServers);
     } else {
       LOG.debug("Group Information found to be null. Some regions might be unassigned.");
-      return Collections.EMPTY_LIST;
+      return Collections.emptyList();
     }
   }
 
@@ -311,17 +311,6 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
     return finalList;
   }
 
-  private ListMultimap<String, HRegionInfo> groupRegions(
-      List<HRegionInfo> regionList) throws IOException {
-    ListMultimap<String, HRegionInfo> regionGroup = ArrayListMultimap
-        .create();
-    for (HRegionInfo region : regionList) {
-      String groupName = RSGroupInfoManager.getRSGroupOfTable(region.getTable());
-      regionGroup.put(groupName, region);
-    }
-    return regionGroup;
-  }
-
   private Set<HRegionInfo> getMisplacedRegions(
       Map<HRegionInfo, ServerName> regions) throws IOException {
     Set<HRegionInfo> misplacedRegions = new HashSet<HRegionInfo>();
@@ -329,13 +318,13 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       HRegionInfo regionInfo = region.getKey();
       ServerName assignedServer = region.getValue();
       RSGroupInfo info =
-          RSGroupInfoManager.getRSGroup(RSGroupInfoManager.getRSGroupOfTable(regionInfo.getTable()));
+          infoManager.getRSGroup(infoManager.getRSGroupOfTable(regionInfo.getTable()));
       if (assignedServer != null &&
           (info == null || !info.containsServer(assignedServer.getAddress()))) {
         LOG.debug("Found misplaced region: " + regionInfo.getRegionNameAsString() +
             " on server: " + assignedServer +
             " found in group: " +
-            RSGroupInfoManager.getRSGroupOfServer(assignedServer.getAddress()) +
+            infoManager.getRSGroupOfServer(assignedServer.getAddress()) +
             " outside of group: " + (info == null ? "UNKNOWN" : info.getName()));
         misplacedRegions.add(regionInfo);
       }
@@ -355,8 +344,8 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
       for (HRegionInfo region : regions) {
         RSGroupInfo info = null;
         try {
-          info = RSGroupInfoManager.getRSGroup(
-              RSGroupInfoManager.getRSGroupOfTable(region.getTable()));
+          info = infoManager.getRSGroup(
+              infoManager.getRSGroupOfTable(region.getTable()));
         } catch (IOException exp) {
           LOG.debug("Group information null for region of table " + region.getTable(),
               exp);
@@ -374,7 +363,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
   @Override
   public void initialize() throws HBaseIOException {
     try {
-      if (RSGroupInfoManager == null) {
+      if (infoManager == null) {
         List<RSGroupAdminEndpoint> cps =
           masterServices.getMasterCoprocessorHost().findCoprocessors(RSGroupAdminEndpoint.class);
         if (cps.size() != 1) {
@@ -382,7 +371,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
           LOG.error(msg);
           throw new HBaseIOException(msg);
         }
-        RSGroupInfoManager = cps.get(0).getGroupInfoManager();
+        infoManager = cps.get(0).getGroupInfoManager();
       }
     } catch (IOException e) {
       throw new HBaseIOException("Failed to initialize GroupInfoManagerImpl", e);
@@ -400,7 +389,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer,
LoadBalanc
   }
 
   public boolean isOnline() {
-    return RSGroupInfoManager != null && RSGroupInfoManager.isOnline();
+    return infoManager != null && infoManager.isOnline();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
----------------------------------------------------------------------
diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
index 60afee0..e7b0e4a 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
@@ -86,8 +86,8 @@ public class RSGroupInfo {
    *
    * @param servers the servers
    */
-  public void addAllServers(Collection<Address> servers){
-    servers.addAll(servers);
+  public void addAllServers(Collection<Address> addresses){
+    this.servers.addAll(addresses);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
----------------------------------------------------------------------
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
index dea0a94..2360ce8 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
@@ -163,33 +163,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<HRegionInfo> regions,
-                                         List<ServerName> servers,
-                                         Map<HRegionInfo, ServerName> assignments)
-      throws IOException {
-    for (HRegionInfo region : regions) {
-      assertTrue(assignments.containsKey(region));
-      ServerName server = assignments.get(region);
-      TableName tableName = region.getTable();
-
-      String groupName =
-          getMockedGroupInfoManager().getRSGroupOfTable(tableName);
-      assertTrue(StringUtils.isNotEmpty(groupName));
-      RSGroupInfo gInfo = getMockedGroupInfoManager().getRSGroup(groupName);
-      assertTrue("Region is not correctly assigned to group servers.",
-          gInfo.containsServer(server.getAddress()));
-    }
-  }
-
-  /**
    * Tests the bulk assignment used during cluster startup.
    *
    * Round-robin. Should yield a balanced cluster so same invariant as the

http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
----------------------------------------------------------------------
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
index a23a430..4c538ec 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
@@ -37,12 +37,12 @@ import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.Waiter;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.constraint.ConstraintException;
+import org.apache.hadoop.hbase.master.RegionState;
 import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.AdminProtos;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -55,6 +55,7 @@ import java.util.Set;
 import java.util.TreeMap;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -121,12 +122,13 @@ public abstract class TestRSGroupsBase {
   }
 
   protected void deleteGroups() throws IOException {
-    RSGroupAdmin groupAdmin = new RSGroupAdminClient(TEST_UTIL.getConnection());
-    for(RSGroupInfo group: groupAdmin.listRSGroups()) {
-      if(!group.getName().equals(RSGroupInfo.DEFAULT_GROUP)) {
-        groupAdmin.moveTables(group.getTables(), RSGroupInfo.DEFAULT_GROUP);
-        groupAdmin.moveServers(group.getServers(), RSGroupInfo.DEFAULT_GROUP);
-        groupAdmin.removeRSGroup(group.getName());
+    try (RSGroupAdmin groupAdmin = new RSGroupAdminClient(TEST_UTIL.getConnection())) {
+      for(RSGroupInfo group: groupAdmin.listRSGroups()) {
+        if(!group.getName().equals(RSGroupInfo.DEFAULT_GROUP)) {
+          groupAdmin.moveTables(group.getTables(), RSGroupInfo.DEFAULT_GROUP);
+          groupAdmin.moveServers(group.getServers(), RSGroupInfo.DEFAULT_GROUP);
+          groupAdmin.removeRSGroup(group.getName());
+        }
       }
     }
   }
@@ -488,6 +490,7 @@ public abstract class TestRSGroupsBase {
         break;
       }
     }
+    assertNotNull(targetServer);
 
     final AdminProtos.AdminService.BlockingInterface targetRS =
         admin.getConnection().getAdmin(targetServer);
@@ -508,10 +511,10 @@ public abstract class TestRSGroupsBase {
     TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate<Exception>() {
       @Override
       public boolean evaluate() throws Exception {
-        return
-            getTableRegionMap().get(tableName) != null &&
-                getTableRegionMap().get(tableName).size() == 6 &&
-                admin.getClusterStatus().getRegionsInTransition().size() < 1;
+        List<String> regions = getTableRegionMap().get(tableName);
+        Set<RegionState> regionsInTransition = admin.getClusterStatus().getRegionsInTransition();
+        return (regions != null && getTableRegionMap().get(tableName).size() == 6)
&&
+           ( regionsInTransition == null || regionsInTransition.size() < 1);
       }
     });
 
@@ -583,7 +586,6 @@ public abstract class TestRSGroupsBase {
         appInfo.getServers().iterator().next().toString());
     AdminProtos.AdminService.BlockingInterface targetRS =
         admin.getConnection().getAdmin(targetServer);
-    HRegionInfo targetRegion = ProtobufUtil.getOnlineRegions(targetRS).get(0);
     Assert.assertEquals(1, ProtobufUtil.getOnlineRegions(targetRS).size());
 
     try {
@@ -779,10 +781,12 @@ public abstract class TestRSGroupsBase {
     TEST_UTIL.waitFor(WAIT_TIMEOUT, new Waiter.Predicate<Exception>() {
       @Override
       public boolean evaluate() throws Exception {
-        return getTableRegionMap().get(tableName) != null &&
-                getTableRegionMap().get(tableName).size() == 5 &&
-                getTableServerRegionMap().get(tableName).size() == 1 &&
-                admin.getClusterStatus().getRegionsInTransition().size() < 1;
+        List<String> regions = getTableRegionMap().get(tableName);
+        Map<ServerName, List<String>> serverMap = getTableServerRegionMap().get(tableName);
+        Set<RegionState> regionsInTransition = admin.getClusterStatus().getRegionsInTransition();
+        return (regions != null && regions.size() == 5) &&
+          (serverMap != null && serverMap.size() == 1) &&
+          (regionsInTransition == null || regionsInTransition.size() < 1);
       }
     });
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/b9475d3e/hbase-shell/src/test/rsgroup/org/apache/hadoop/hbase/client/rsgroup/TestShellRSGroups.java
----------------------------------------------------------------------
diff --git a/hbase-shell/src/test/rsgroup/org/apache/hadoop/hbase/client/rsgroup/TestShellRSGroups.java
b/hbase-shell/src/test/rsgroup/org/apache/hadoop/hbase/client/rsgroup/TestShellRSGroups.java
index be23a59..de10529 100644
--- a/hbase-shell/src/test/rsgroup/org/apache/hadoop/hbase/client/rsgroup/TestShellRSGroups.java
+++ b/hbase-shell/src/test/rsgroup/org/apache/hadoop/hbase/client/rsgroup/TestShellRSGroups.java
@@ -80,7 +80,7 @@ public class TestShellRSGroups {
     TEST_UTIL.startMiniCluster(1,4);
 
     // Configure jruby runtime
-    List<String> loadPaths = new ArrayList();
+    List<String> loadPaths = new ArrayList<>();
     loadPaths.add(basePath+"/src/main/ruby");
     loadPaths.add(basePath+"/src/test/ruby");
     jruby.getProvider().setLoadPaths(loadPaths);


Mime
View raw message