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/e95f94a7 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e95f94a7 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e95f94a7 Branch: refs/heads/branch-1 Commit: e95f94a72dd9b550ccfea213644bf3130f61e65c Parents: d80d3fa Author: Andrew Purtell <apurt...@apache.org> Authored: Wed Nov 15 18:47:45 2017 -0800 Committer: Andrew Purtell <apurt...@apache.org> Committed: Fri Nov 17 17:12:36 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/e95f94a7/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/e95f94a7/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/e95f94a7/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/e95f94a7/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/e95f94a7/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/e95f94a7/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);