HBASE-18575 [AMv2] Fixed and enabled TestRestartCluster#testRetainAssignmentOnRestart on master

* Fixed ServerCrashProcedure to set forceNewPlan to false for instances AssignProcedure. This enables balancer to find most suitable target server
* Fixed and enabled TestRestartCluster#testRetainAssignmentOnRestart on master
* Renamed method ServerName@isSameHostnameAndPort() to isSameAddress()

Signed-off-by: Michael Stack <stack@apache.org>
This commit is contained in:
Umesh Agashe 2017-08-22 16:23:21 -07:00 committed by Michael Stack
parent 37c6594627
commit fad968d99f
19 changed files with 41 additions and 42 deletions

View File

@ -347,8 +347,8 @@ public class ServerName implements Comparable<ServerName>, Serializable {
* @param right
* @return True if <code>other</code> has same hostname and port.
*/
public static boolean isSameHostnameAndPort(final ServerName left,
final ServerName right) {
public static boolean isSameAddress(final ServerName left,
final ServerName right) {
// TODO: Make this left.getAddress().equals(right.getAddress())
if (left == null) return false;
if (right == null) return false;

View File

@ -320,7 +320,7 @@ public class DistributedHBaseCluster extends HBaseCluster {
List<IOException> deferred = new ArrayList<>();
//check whether current master has changed
final ServerName initMaster = initial.getMaster();
if (!ServerName.isSameHostnameAndPort(initMaster, current.getMaster())) {
if (!ServerName.isSameAddress(initMaster, current.getMaster())) {
LOG.info("Restoring cluster - Initial active master : "
+ initMaster.getHostAndPort()
+ " has changed to : "
@ -340,7 +340,7 @@ public class DistributedHBaseCluster extends HBaseCluster {
// 2. Stop current master
// 3. Start backup masters
for (ServerName currentBackup : current.getBackupMasters()) {
if (!ServerName.isSameHostnameAndPort(currentBackup, initMaster)) {
if (!ServerName.isSameAddress(currentBackup, initMaster)) {
LOG.info("Restoring cluster - stopping backup master: " + currentBackup);
stopMaster(currentBackup);
}

View File

@ -420,7 +420,7 @@ public class TestRSGroupBasedLoadBalancer {
ServerAndLoad newSAL = null;
ServerAndLoad oldSAL = null;
for (ServerAndLoad sal : previousLoad.get(groupName)) {
if (ServerName.isSameHostnameAndPort(sn, sal.getServerName())) {
if (ServerName.isSameAddress(sn, sal.getServerName())) {
oldSAL = sal;
newSAL = new ServerAndLoad(sn, sal.getLoad() + diff);
break;

View File

@ -31,7 +31,6 @@ import java.util.Map.Entry;
import java.util.Random;
import java.util.Set;
import org.apache.hadoop.hbase.shaded.com.google.common.collect.Maps;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@ -105,7 +104,7 @@ public class FavoredNodeAssignmentHelper {
this.rackToRegionServerMap.put(rackName, serverList);
}
for (ServerName serverName : serverList) {
if (ServerName.isSameHostnameAndPort(sn, serverName)) {
if (ServerName.isSameAddress(sn, serverName)) {
// The server is already present, ignore.
break;
}

View File

@ -278,7 +278,7 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored
// server with the legit startcode
private ServerName availableServersContains(List<ServerName> servers, ServerName favoredNode) {
for (ServerName server : servers) {
if (ServerName.isSameHostnameAndPort(favoredNode, server)) {
if (ServerName.isSameAddress(favoredNode, server)) {
return server;
}
}

View File

@ -92,7 +92,7 @@ public class FavoredNodesPlan {
return null;
}
for (Position p : Position.values()) {
if (ServerName.isSameHostnameAndPort(favoredNodes.get(p.ordinal()),server)) {
if (ServerName.isSameAddress(favoredNodes.get(p.ordinal()),server)) {
return p;
}
}

View File

@ -200,7 +200,7 @@ public class ActiveMasterManager extends ZooKeeperListener {
// Hopefully next time around we won't fail the parse. Dangerous.
continue;
}
if (ServerName.isSameHostnameAndPort(currentMaster, this.sn)) {
if (ServerName.isSameAddress(currentMaster, this.sn)) {
msg = ("Current master has this master's address, " +
currentMaster + "; master was restarted? Deleting node.");
// Hurry along the expiration of the znode.

View File

@ -75,7 +75,7 @@ public class DeadServer {
Iterator<ServerName> it = deadServers.keySet().iterator();
while (it.hasNext()) {
ServerName sn = it.next();
if (ServerName.isSameHostnameAndPort(sn, newServerName)) {
if (ServerName.isSameAddress(sn, newServerName)) {
it.remove();
return true;
}
@ -149,7 +149,7 @@ public class DeadServer {
Iterator<ServerName> it = deadServers.keySet().iterator();
while (it.hasNext()) {
ServerName sn = it.next();
if (ServerName.isSameHostnameAndPort(sn, newServerName)) {
if (ServerName.isSameAddress(sn, newServerName)) {
it.remove();
}
}

View File

@ -55,7 +55,6 @@ import org.apache.hadoop.hbase.client.ClusterConnection;
import org.apache.hadoop.hbase.client.RetriesExhaustedException;
import org.apache.hadoop.hbase.ipc.HBaseRpcController;
import org.apache.hadoop.hbase.ipc.RpcControllerFactory;
import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer;
import org.apache.hadoop.hbase.monitoring.MonitoredTask;
import org.apache.hadoop.hbase.regionserver.HRegionServer;
import org.apache.hadoop.hbase.shaded.com.google.protobuf.UnsafeByteOperations;
@ -414,7 +413,7 @@ public class ServerManager {
ServerName r = onlineServers.lowerKey(end);
if (r != null) {
if (ServerName.isSameHostnameAndPort(r, serverName)) {
if (ServerName.isSameAddress(r, serverName)) {
return r;
}
}

View File

@ -252,7 +252,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements
*/
private ServerName getServerFromFavoredNode(List<ServerName> servers, ServerName fn) {
for (ServerName server : servers) {
if (ServerName.isSameHostnameAndPort(fn, server)) {
if (ServerName.isSameAddress(fn, server)) {
return server;
}
}
@ -463,7 +463,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements
List<ServerName> result = Lists.newArrayList();
for (ServerName sn : serversWithoutStartCodes) {
for (ServerName online : onlineServers) {
if (ServerName.isSameHostnameAndPort(sn, online)) {
if (ServerName.isSameAddress(sn, online)) {
result.add(online);
}
}

View File

@ -164,8 +164,10 @@ implements ServerProcedureInterface {
}
handleRIT(env, regionsOnCrashedServer);
AssignmentManager am = env.getAssignmentManager();
// forceNewPlan is set to false. Balancer is expected to find most suitable target
// server if retention is not possible.
addChildProcedure(am.
createAssignProcedures(am.getOrderedRegions(regionsOnCrashedServer), true));
createAssignProcedures(am.getOrderedRegions(regionsOnCrashedServer), false));
}
setNextState(ServerCrashState.SERVER_CRASH_FINISH);
break;

View File

@ -118,7 +118,6 @@ import org.apache.hadoop.hbase.ipc.ServerRpcController;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.LoadBalancer;
import org.apache.hadoop.hbase.master.RegionState.State;
import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer;
import org.apache.hadoop.hbase.mob.MobCacheConfig;
import org.apache.hadoop.hbase.procedure.RegionServerProcedureManagerHost;
import org.apache.hadoop.hbase.quotas.FileSystemUtilizationChore;
@ -3406,7 +3405,7 @@ public class HRegionServer extends HasThread implements
private static final int TIMEOUT_REGION_MOVED = (2 * 60 * 1000);
protected void addToMovedRegions(String encodedName, ServerName destination, long closeSeqNum) {
if (ServerName.isSameHostnameAndPort(destination, this.getServerName())) {
if (ServerName.isSameAddress(destination, this.getServerName())) {
LOG.warn("Not adding moved region record: " + encodedName + " to self.");
return;
}

View File

@ -108,7 +108,7 @@ public class TestServerName {
assertEquals(lower.hashCode(), upper.hashCode());
assertTrue(lower.equals(upper));
assertTrue(upper.equals(lower));
assertTrue(ServerName.isSameHostnameAndPort(lower, upper));
assertTrue(ServerName.isSameAddress(lower, upper));
}
}

View File

@ -35,14 +35,14 @@ public class TestStartcodeAgnosticServerName {
StartcodeAgnosticServerName snStartCode =
new StartcodeAgnosticServerName("www.example.org", 1234, 5678);
assertTrue(ServerName.isSameHostnameAndPort(sn, snStartCode));
assertTrue(ServerName.isSameAddress(sn, snStartCode));
assertTrue(snStartCode.equals(sn));
assertTrue(sn.equals(snStartCode));
assertEquals(0, snStartCode.compareTo(sn));
StartcodeAgnosticServerName snStartCodeFNPort =
new StartcodeAgnosticServerName("www.example.org", 1234, ServerName.NON_STARTCODE);
assertTrue(ServerName.isSameHostnameAndPort(snStartCodeFNPort, snStartCode));
assertTrue(ServerName.isSameAddress(snStartCodeFNPort, snStartCode));
assertTrue(snStartCode.equals(snStartCodeFNPort));
assertTrue(snStartCodeFNPort.equals(snStartCode));
assertEquals(0, snStartCode.compareTo(snStartCodeFNPort));

View File

@ -201,14 +201,14 @@ public class TestRegionPlacement {
break;
}
}
} while (ServerName.isSameHostnameAndPort(metaServer, serverToKill) || isNamespaceServer ||
} while (ServerName.isSameAddress(metaServer, serverToKill) || isNamespaceServer ||
TEST_UTIL.getHBaseCluster().getRegionServer(killIndex).getNumberOfOnlineRegions() == 0);
LOG.debug("Stopping RS " + serverToKill);
Map<HRegionInfo, Pair<ServerName, ServerName>> regionsToVerify = new HashMap<>();
// mark the regions to track
for (Map.Entry<HRegionInfo, ServerName[]> entry : favoredNodesAssignmentPlan.entrySet()) {
ServerName s = entry.getValue()[0];
if (ServerName.isSameHostnameAndPort(s, serverToKill)) {
if (ServerName.isSameAddress(s, serverToKill)) {
regionsToVerify.put(entry.getKey(), new Pair<>(entry.getValue()[1], entry.getValue()[2]));
LOG.debug("Adding " + entry.getKey() + " with sedcondary/tertiary " +
entry.getValue()[1] + " " + entry.getValue()[2]);
@ -232,8 +232,8 @@ public class TestRegionPlacement {
LOG.debug("New destination for region " + entry.getKey().getEncodedName() +
" " + newDestination +". Secondary/Tertiary are " + secondaryTertiaryServers.getFirst()
+ "/" + secondaryTertiaryServers.getSecond());
if (!(ServerName.isSameHostnameAndPort(newDestination, secondaryTertiaryServers.getFirst())||
ServerName.isSameHostnameAndPort(newDestination, secondaryTertiaryServers.getSecond()))){
if (!(ServerName.isSameAddress(newDestination, secondaryTertiaryServers.getFirst())||
ServerName.isSameAddress(newDestination, secondaryTertiaryServers.getSecond()))){
fail("Region " + entry.getKey() + " not present on any of the expected servers");
}
}

View File

@ -94,7 +94,7 @@ public class TestRegionPlacement2 {
((FavoredNodeLoadBalancer)balancer).getFavoredNodes(region);
assertTrue(favoredNodesBefore.size() == FavoredNodeAssignmentHelper.FAVORED_NODES_NUM);
// the primary RS should be the one that the balancer's assignment returns
assertTrue(ServerName.isSameHostnameAndPort(serverBefore.iterator().next(),
assertTrue(ServerName.isSameAddress(serverBefore.iterator().next(),
favoredNodesBefore.get(PRIMARY)));
// now remove the primary from the list of available servers
List<ServerName> removedServers = removeMatchingServers(serverBefore, servers);
@ -110,9 +110,9 @@ public class TestRegionPlacement2 {
Set<ServerName> serverAfter = assignmentMap.keySet();
// We expect the new RegionServer assignee to be one of the favored nodes
// chosen earlier.
assertTrue(ServerName.isSameHostnameAndPort(serverAfter.iterator().next(),
assertTrue(ServerName.isSameAddress(serverAfter.iterator().next(),
favoredNodesBefore.get(SECONDARY)) ||
ServerName.isSameHostnameAndPort(serverAfter.iterator().next(),
ServerName.isSameAddress(serverAfter.iterator().next(),
favoredNodesBefore.get(TERTIARY)));
// put back the primary in the list of available servers
@ -153,7 +153,7 @@ public class TestRegionPlacement2 {
((FavoredNodeLoadBalancer)balancer).getFavoredNodes(region);
assertTrue(favoredNodesBefore.size() == FavoredNodeAssignmentHelper.FAVORED_NODES_NUM);
// the primary RS should be the one that the balancer's assignment returns
assertTrue(ServerName.isSameHostnameAndPort(serverBefore,favoredNodesBefore.get(PRIMARY)));
assertTrue(ServerName.isSameAddress(serverBefore,favoredNodesBefore.get(PRIMARY)));
// now remove the primary from the list of servers
removeMatchingServers(serverBefore, servers);
// call randomAssignment with the modified servers list
@ -167,8 +167,8 @@ public class TestRegionPlacement2 {
assertTrue(favoredNodesAfter.containsAll(favoredNodesBefore));
// We expect the new RegionServer assignee to be one of the favored nodes
// chosen earlier.
assertTrue(ServerName.isSameHostnameAndPort(serverAfter, favoredNodesBefore.get(SECONDARY)) ||
ServerName.isSameHostnameAndPort(serverAfter, favoredNodesBefore.get(TERTIARY)));
assertTrue(ServerName.isSameAddress(serverAfter, favoredNodesBefore.get(SECONDARY)) ||
ServerName.isSameAddress(serverAfter, favoredNodesBefore.get(TERTIARY)));
// Make all the favored nodes unavailable for assignment
removeMatchingServers(favoredNodesAfter, servers);
// call randomAssignment with the modified servers list
@ -194,7 +194,7 @@ public class TestRegionPlacement2 {
List<ServerName> servers) {
List<ServerName> serversToRemove = new ArrayList<>();
for (ServerName s : servers) {
if (ServerName.isSameHostnameAndPort(s, serverWithoutStartCode)) {
if (ServerName.isSameAddress(s, serverWithoutStartCode)) {
serversToRemove.add(s);
}
}

View File

@ -36,6 +36,8 @@ import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableExistsException;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.master.assignment.RegionStates;
import org.apache.hadoop.hbase.net.Address;
import org.apache.hadoop.hbase.shaded.com.google.common.net.HostAndPort;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.util.Bytes;
@ -109,7 +111,6 @@ public class TestRestartCluster {
* This tests retaining assignments on a cluster restart
*/
@Test (timeout=300000)
@Ignore // Does not work in new AMv2 currently.
public void testRetainAssignmentOnRestart() throws Exception {
UTIL.startMiniCluster(2);
while (!UTIL.getMiniHBaseCluster().getMaster().isInitialized()) {
@ -163,7 +164,7 @@ public class TestRestartCluster {
LOG.info("\n\nStarting cluster the second time with the same ports");
try {
cluster.getConf().setInt(
ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, 4);
ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, 3);
master = cluster.startMaster().getMaster();
for (int i = 0; i < 3; i++) {
cluster.getConf().setInt(HConstants.REGIONSERVER_PORT, rsPorts[i]);
@ -178,7 +179,7 @@ public class TestRestartCluster {
// Make sure live regionservers are on the same host/port
List<ServerName> localServers = master.getServerManager().getOnlineServersList();
assertEquals(4, localServers.size());
assertEquals(3, localServers.size());
for (int i = 0; i < 3; i++) {
boolean found = false;
for (ServerName serverName: localServers) {

View File

@ -133,7 +133,7 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase {
Map<ServerName, List<HRegionInfo>> serverAssignments = Maps.newHashMap();
ClusterStatus status = admin.getClusterStatus();
for (ServerName sn : status.getServers()) {
if (!ServerName.isSameHostnameAndPort(sn, masterServerName)) {
if (!ServerName.isSameAddress(sn, masterServerName)) {
serverAssignments.put(sn, admin.getOnlineRegions(sn));
}
}
@ -195,7 +195,7 @@ public class TestFavoredStochasticBalancerPickers extends BalancerTestBase {
private boolean doesMatchExcludeNodes(ArrayList<ServerName> excludeNodes, ServerName sn) {
for (ServerName excludeSN : excludeNodes) {
if (ServerName.isSameHostnameAndPort(sn, excludeSN)) {
if (ServerName.isSameAddress(sn, excludeSN)) {
return true;
}
}

View File

@ -44,7 +44,6 @@ import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.favored.FavoredNodeAssignmentHelper;
import org.apache.hadoop.hbase.favored.FavoredNodesPlan;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.master.assignment.RegionStates;
import org.apache.hadoop.hbase.master.assignment.RegionStates.RegionStateNode;
import org.apache.hadoop.hbase.master.ServerManager;
@ -220,7 +219,7 @@ public class TestFavoredStochasticLoadBalancer extends BalancerTestBase {
assertNotNull(favoredNodes);
boolean containsFN = false;
for (ServerName sn : favoredNodes) {
if (ServerName.isSameHostnameAndPort(destination, sn)) {
if (ServerName.isSameAddress(destination, sn)) {
containsFN = true;
}
}
@ -305,7 +304,7 @@ public class TestFavoredStochasticLoadBalancer extends BalancerTestBase {
@Override
public boolean evaluate() throws Exception {
ServerName host = regionStates.getRegionServerOfRegion(misplacedRegion);
return !ServerName.isSameHostnameAndPort(host, current);
return !ServerName.isSameAddress(host, current);
}
});
checkFavoredNodeAssignments(tableName, fnm, regionStates);
@ -516,7 +515,7 @@ public class TestFavoredStochasticLoadBalancer extends BalancerTestBase {
private void stopServersAndWaitUntilProcessed(List<ServerName> currentFN) throws Exception {
for (ServerName sn : currentFN) {
for (JVMClusterUtil.RegionServerThread rst : cluster.getLiveRegionServerThreads()) {
if (ServerName.isSameHostnameAndPort(sn, rst.getRegionServer().getServerName())) {
if (ServerName.isSameAddress(sn, rst.getRegionServer().getServerName())) {
LOG.info("Shutting down server: " + sn);
cluster.stopRegionServer(rst.getRegionServer().getServerName());
cluster.waitForRegionServerToStop(rst.getRegionServer().getServerName(), 60000);