HBASE-10632 Region lost in limbo after ArrayIndexOutOfBoundsException during assignment

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1573723 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Enis Soztutar 2014-03-03 20:07:04 +00:00
parent 0e0f813e07
commit e196b203ac
3 changed files with 121 additions and 5 deletions

View File

@ -19,17 +19,16 @@ package org.apache.hadoop.hbase.master.balancer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.Deque;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.NavigableMap;
import java.util.Random;
import java.util.Set;
import java.util.TreeMap;
import java.util.NavigableMap;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@ -140,7 +139,13 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
servers[serverIndex] = entry.getKey();
}
regionsPerServer[serverIndex] = new int[entry.getValue().size()];
if (regionsPerServer[serverIndex] != null) {
// there is another server with the same hostAndPort in ClusterState.
// allocate the array for the total size
regionsPerServer[serverIndex] = new int[entry.getValue().size() + regionsPerServer[serverIndex].length];
} else {
regionsPerServer[serverIndex] = new int[entry.getValue().size()];
}
serverIndicesSortedByRegionCount[serverIndex] = serverIndex;
}
@ -181,7 +186,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
for (int i=0; i < loc.size(); i++) {
regionLocations[regionIndex][i] =
loc.get(i) == null ? -1 :
(serversToIndex.get(loc.get(i)) == null ? -1 : serversToIndex.get(loc.get(i)));
(serversToIndex.get(loc.get(i).getHostAndPort()) == null ? -1 : serversToIndex.get(loc.get(i).getHostAndPort()));
}
}
@ -369,10 +374,12 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
return this.config;
}
@Override
public void setClusterStatus(ClusterStatus st) {
// Not used except for the StocasticBalancer
}
@Override
public void setMasterServices(MasterServices masterServices) {
this.services = masterServices;
}
@ -422,6 +429,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
* @return map of server to the regions it should take, or null if no
* assignment is possible (ie. no regions or no servers)
*/
@Override
public Map<ServerName, List<HRegionInfo>> roundRobinAssignment(List<HRegionInfo> regions,
List<ServerName> servers) {
metricsBalancer.incrMiscInvocations();
@ -467,6 +475,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
* @param servers
* @return map of regions to the server it should be assigned to
*/
@Override
public Map<HRegionInfo, ServerName> immediateAssignment(List<HRegionInfo> regions,
List<ServerName> servers) {
metricsBalancer.incrMiscInvocations();
@ -481,6 +490,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
/**
* Used to assign a single region to a random server.
*/
@Override
public ServerName randomAssignment(HRegionInfo regionInfo, List<ServerName> servers) {
metricsBalancer.incrMiscInvocations();
@ -508,6 +518,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
* @param servers available servers
* @return map of servers and regions to be assigned to them
*/
@Override
public Map<ServerName, List<HRegionInfo>> retainAssignment(Map<HRegionInfo, ServerName> regions,
List<ServerName> servers) {
// Update metrics

View File

@ -549,7 +549,9 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
idx++;
}
return idx;
return idx < regionLocations.length
? regionLocations[idx]
: pickOtherRandomServer(cluster, thisServer);
}
void setServices(MasterServices services) {
@ -824,6 +826,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
}
@Override
double cost(Cluster cluster) {
if (clusterStatus == null || loads == null) {
return 0;
@ -891,6 +894,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
}
@Override
protected double getCostFromRl(RegionLoad rl) {
return rl.getReadRequestsCount();
}
@ -911,6 +915,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer {
this.setMultiplier(conf.getFloat(WRITE_REQUEST_COST_KEY, DEFAULT_WRITE_REQUEST_COST));
}
@Override
protected double getCostFromRl(RegionLoad rl) {
return rl.getWriteRequestsCount();
}

View File

@ -19,14 +19,18 @@ package org.apache.hadoop.hbase.master.balancer;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.conf.Configuration;
@ -36,10 +40,13 @@ import org.apache.hadoop.hbase.MediumTests;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.master.LoadBalancer;
import org.apache.hadoop.hbase.master.RegionPlan;
import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer.Cluster;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import com.google.common.collect.Lists;
@Category(MediumTests.class)
public class TestBaseLoadBalancer extends BalancerTestBase {
@ -228,4 +235,97 @@ public class TestBaseLoadBalancer extends BalancerTestBase {
}
}
@Test
public void testClusterServersWithSameHostPort() {
// tests whether the BaseLoadBalancer.Cluster can be constructed with servers
// sharing same host and port
List<ServerName> servers = getListOfServerNames(randomServers(10, 10));
List<HRegionInfo> regions = randomRegions(101);
Map<ServerName, List<HRegionInfo>> clusterState = new HashMap<ServerName, List<HRegionInfo>>();
assignRegions(regions, servers, clusterState);
// construct another list of servers, but sharing same hosts and ports
List<ServerName> oldServers = new ArrayList<ServerName>(servers.size());
for (ServerName sn : servers) {
// The old server would have had same host and port, but different start code!
oldServers.add(ServerName.valueOf(sn.getHostname(), sn.getPort(), sn.getStartcode() - 10));
}
regions = randomRegions(9); // some more regions
assignRegions(regions, oldServers, clusterState);
// should not throw exception:
BaseLoadBalancer.Cluster cluster = new Cluster(clusterState, null, null);
assertEquals(101 + 9, cluster.numRegions);
assertEquals(10, cluster.numServers); // only 10 servers because they share the same host + port
}
private void assignRegions(List<HRegionInfo> regions, List<ServerName> servers,
Map<ServerName, List<HRegionInfo>> clusterState) {
for (int i = 0; i < regions.size(); i++) {
ServerName sn = servers.get(i % servers.size());
List<HRegionInfo> regionsOfServer = clusterState.get(sn);
if (regionsOfServer == null) {
regionsOfServer = new ArrayList<HRegionInfo>(10);
clusterState.put(sn, regionsOfServer);
}
regionsOfServer.add(regions.get(i));
}
}
@Test
public void testClusterRegionLocations() {
// tests whether region locations are handled correctly in Cluster
List<ServerName> servers = getListOfServerNames(randomServers(10, 10));
List<HRegionInfo> regions = randomRegions(101);
Map<ServerName, List<HRegionInfo>> clusterState = new HashMap<ServerName, List<HRegionInfo>>();
assignRegions(regions, servers, clusterState);
// mock block locality for some regions
RegionLocationFinder locationFinder = mock(RegionLocationFinder.class);
// block locality: region:0 => {server:0}
// region:1 => {server:0, server:1}
// region:42 => {server:4, server:9, server:5}
when(locationFinder.getTopBlockLocations(regions.get(0))).thenReturn(
Lists.newArrayList(servers.get(0)));
when(locationFinder.getTopBlockLocations(regions.get(1))).thenReturn(
Lists.newArrayList(servers.get(0), servers.get(1)));
when(locationFinder.getTopBlockLocations(regions.get(42))).thenReturn(
Lists.newArrayList(servers.get(4), servers.get(9), servers.get(5)));
BaseLoadBalancer.Cluster cluster = new Cluster(clusterState, null, locationFinder);
int r0 = ArrayUtils.indexOf(cluster.regions, regions.get(0)); // this is ok, it is just a test
int r1 = ArrayUtils.indexOf(cluster.regions, regions.get(1));
int r10 = ArrayUtils.indexOf(cluster.regions, regions.get(10));
int r42 = ArrayUtils.indexOf(cluster.regions, regions.get(42));
int s0 = cluster.serversToIndex.get(servers.get(0).getHostAndPort());
int s1 = cluster.serversToIndex.get(servers.get(1).getHostAndPort());
int s4 = cluster.serversToIndex.get(servers.get(4).getHostAndPort());
int s5 = cluster.serversToIndex.get(servers.get(5).getHostAndPort());
int s9 = cluster.serversToIndex.get(servers.get(9).getHostAndPort());
// region 0 locations
assertEquals(1, cluster.regionLocations[r0].length);
assertEquals(s0, cluster.regionLocations[r0][0]);
// region 1 locations
assertEquals(2, cluster.regionLocations[r1].length);
assertEquals(s0, cluster.regionLocations[r1][0]);
assertEquals(s1, cluster.regionLocations[r1][1]);
// region 10 locations
assertEquals(0, cluster.regionLocations[r10].length);
// region 42 locations
assertEquals(3, cluster.regionLocations[r42].length);
assertEquals(s4, cluster.regionLocations[r42][0]);
assertEquals(s9, cluster.regionLocations[r42][1]);
assertEquals(s5, cluster.regionLocations[r42][2]);
}
}