HBASE-19239 Fix findbugs and error-prone issues

Fixes for hbase-rsgroup
This commit is contained in:
Andrew Purtell 2017-11-15 18:47:45 -08:00
parent d80d3fa454
commit e95f94a72d
6 changed files with 44 additions and 78 deletions

View File

@ -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);

View File

@ -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

View File

@ -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);
}
/**

View File

@ -162,33 +162,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.
*

View File

@ -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);
}
});

View File

@ -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);