You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2017/11/18 01:22:47 UTC
[19/30] hbase git commit: HBASE-19239 Fix findbugs and error-prone
issues
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 <ap...@apache.org>
Authored: Wed Nov 15 18:47:45 2017 -0800
Committer: Andrew Purtell <ap...@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);