From 7bcdd44be6951aae3f3e041b682b7d06c06f1721 Mon Sep 17 00:00:00 2001 From: Matteo Bertozzi Date: Fri, 3 Jun 2016 12:41:51 -0700 Subject: [PATCH] HBASE-15927 Remove HMaster.assignRegion() --- .../apache/hadoop/hbase/master/HMaster.java | 4 -- .../hbase/master/MasterRpcServices.java | 4 +- .../hadoop/hbase/HBaseTestingUtility.java | 15 ++++++ .../hbase/client/TestMetaWithReplicas.java | 2 +- .../TestAssignmentManagerOnCluster.java | 50 ++++++++----------- .../hbase/master/TestMasterFailover.java | 36 ++++++------- .../hadoop/hbase/util/TestHBaseFsck.java | 35 ++++--------- 7 files changed, 67 insertions(+), 79 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index beb1bcaa35e..4b7e18142fa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -2387,10 +2387,6 @@ public class HMaster extends HRegionServer implements MasterServices, Server { return this.initializationBeforeMetaAssignment; } - public void assignRegion(HRegionInfo hri) { - assignmentManager.assign(hri, true); - } - /** * Compute the average load across all region servers. * Currently, this uses a very naive computation - just uses the number of diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 90471cf6af5..152379d26fd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -1389,7 +1389,7 @@ public class MasterRpcServices extends RSRpcServices if (master.assignmentManager.getRegionStates().isRegionOffline(hri)) { LOG.debug("Region " + hri.getRegionNameAsString() + " is not online on any region server, reassigning it."); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } if (master.cpHost != null) { master.cpHost.postUnassign(hri, force); @@ -1572,7 +1572,7 @@ public class MasterRpcServices extends RSRpcServices } } - /** + /** * Returns the security capabilities in effect on the cluster */ @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index 2870593d406..4547e0fc4ca 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -3387,6 +3387,21 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility { } } + /** + * Uses directly the assignment manager to assign the region. + * and waits until the specified region has completed assignment. + * @param tableName the table name + * @throws IOException + * @throw InterruptedException + * @return true if the region is assigned false otherwise. + */ + public boolean assignRegion(final HRegionInfo regionInfo) + throws IOException, InterruptedException { + final AssignmentManager am = getHBaseCluster().getMaster().getAssignmentManager(); + am.assign(regionInfo, true); + return am.waitForAssignment(regionInfo); + } + /** * Wait until all regions for a table in hbase:meta have a non-empty * info:server, up to a configuable timeout value (default is 60 seconds) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaWithReplicas.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaWithReplicas.java index d0f6a97112b..0ef989052b1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaWithReplicas.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaWithReplicas.java @@ -420,7 +420,7 @@ public class TestMetaWithReplicas { // create in-memory state otherwise master won't assign TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager() .getRegionStates().createRegionState(h); - TEST_UTIL.getMiniHBaseCluster().getMaster().assignRegion(h); + TEST_UTIL.assignRegion(h); HBaseFsckRepair.waitUntilAssigned(TEST_UTIL.getHBaseAdmin(), h); // check that problem exists HBaseFsck hbck = doFsck(TEST_UTIL.getConfiguration(), false); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java index 343cd4cf61b..7be46f5a46c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java @@ -198,9 +198,8 @@ public class TestAssignmentManagerOnCluster { MetaTableAccessor.addRegionToMeta(meta, hri); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - am.waitForAssignment(hri); + TEST_UTIL.assignRegion(hri); RegionStates regionStates = am.getRegionStates(); ServerName serverName = regionStates.getRegionServerOfRegion(hri); @@ -308,7 +307,7 @@ public class TestAssignmentManagerOnCluster { final AssignmentManager am = master.getAssignmentManager(); RegionPlan plan = new RegionPlan(hri, null, deadServer); am.addPlan(hri.getEncodedName(), plan); - master.assignRegion(hri); + TEST_UTIL.assignRegion(hri); int version = ZKAssign.transitionNode(master.getZooKeeper(), hri, destServer, EventType.M_ZK_REGION_OFFLINE, @@ -519,9 +518,8 @@ public class TestAssignmentManagerOnCluster { MetaTableAccessor.addRegionToMeta(meta, hri); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); ServerName sn = am.getRegionStates().getRegionServerOfRegion(hri); TEST_UTIL.assertRegionOnServer(hri, sn, 6000); @@ -568,9 +566,8 @@ public class TestAssignmentManagerOnCluster { MetaTableAccessor.addRegionToMeta(meta, hri); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); ServerName sn = am.getRegionStates().getRegionServerOfRegion(hri); TEST_UTIL.assertRegionOnServer(hri, sn, 6000); @@ -616,9 +613,8 @@ public class TestAssignmentManagerOnCluster { MyLoadBalancer.controledRegion = hri.getEncodedName(); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - assertFalse(am.waitForAssignment(hri)); + assertFalse(TEST_UTIL.assignRegion(hri)); RegionState state = am.getRegionStates().getRegionState(hri); assertEquals(RegionState.State.FAILED_OPEN, state.getState()); @@ -626,8 +622,7 @@ public class TestAssignmentManagerOnCluster { assertNull(state.getServerName()); MyLoadBalancer.controledRegion = null; - master.assignRegion(hri); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); ServerName serverName = master.getAssignmentManager(). getRegionStates().getRegionServerOfRegion(hri); @@ -663,9 +658,8 @@ public class TestAssignmentManagerOnCluster { fs.create(regionDir, true); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - assertFalse(am.waitForAssignment(hri)); + assertFalse(TEST_UTIL.assignRegion(hri)); RegionState state = am.getRegionStates().getRegionState(hri); assertEquals(RegionState.State.FAILED_OPEN, state.getState()); @@ -676,8 +670,7 @@ public class TestAssignmentManagerOnCluster { // remove the blocking file, so that region can be opened fs.delete(regionDir, true); - master.assignRegion(hri); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); ServerName serverName = master.getAssignmentManager(). getRegionStates().getRegionServerOfRegion(hri); @@ -754,9 +747,8 @@ public class TestAssignmentManagerOnCluster { MetaTableAccessor.addRegionToMeta(meta, hri); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); ServerName sn = am.getRegionStates().getRegionServerOfRegion(hri); TEST_UTIL.assertRegionOnServer(hri, sn, 6000); @@ -811,8 +803,9 @@ public class TestAssignmentManagerOnCluster { MyRegionObserver.postOpenEnabled.set(true); MyRegionObserver.postOpenCalled = false; HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); + AssignmentManager am = master.getAssignmentManager(); // Region will be opened, but it won't complete - master.assignRegion(hri); + am.assign(hri, true); long end = EnvironmentEdgeManager.currentTime() + 20000; // Wait till postOpen is called while (!MyRegionObserver.postOpenCalled ) { @@ -821,7 +814,6 @@ public class TestAssignmentManagerOnCluster { Thread.sleep(300); } - AssignmentManager am = master.getAssignmentManager(); // Now let's unassign it, it should do nothing am.unassign(hri); RegionState state = am.getRegionStates().getRegionState(hri); @@ -884,12 +876,14 @@ public class TestAssignmentManagerOnCluster { // Assign the region master = (MyMaster)cluster.getMaster(); - master.assignRegion(hri); + AssignmentManager am = master.getAssignmentManager(); + + am.assign(hri, true); // Hold SSH before killing the hosting server master.enableSSH(false); - AssignmentManager am = master.getAssignmentManager(); + RegionStates regionStates = am.getRegionStates(); ServerName metaServer = regionStates.getRegionServerOfRegion( HRegionInfo.FIRST_META_REGIONINFO); @@ -959,10 +953,9 @@ public class TestAssignmentManagerOnCluster { // Assign the region master = (MyMaster)cluster.getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); RegionStates regionStates = am.getRegionStates(); - assertTrue(am.waitForAssignment(hri)); + assertTrue(TEST_UTIL.assignRegion(hri)); // Disable the table admin.disableTable(table); @@ -1000,9 +993,9 @@ public class TestAssignmentManagerOnCluster { // Assign the region master = (MyMaster)cluster.getMaster(); - master.assignRegion(hri); - AssignmentManager am = master.getAssignmentManager(); + am.assign(hri, true); + RegionStates regionStates = am.getRegionStates(); ServerName metaServer = regionStates.getRegionServerOfRegion( HRegionInfo.FIRST_META_REGIONINFO); @@ -1125,9 +1118,9 @@ public class TestAssignmentManagerOnCluster { // Assign the region master = (MyMaster)cluster.getMaster(); - master.assignRegion(hri); - AssignmentManager am = master.getAssignmentManager(); + am.assign(hri, true); + RegionStates regionStates = am.getRegionStates(); ServerName metaServer = regionStates.getRegionServerOfRegion( HRegionInfo.FIRST_META_REGIONINFO); @@ -1192,9 +1185,8 @@ public class TestAssignmentManagerOnCluster { new HRegionInfo(desc.getTableName(), Bytes.toBytes("A"), Bytes.toBytes("Z")); MetaTableAccessor.addRegionToMeta(meta, hri); HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - master.assignRegion(hri); AssignmentManager am = master.getAssignmentManager(); - am.waitForAssignment(hri); + TEST_UTIL.assignRegion(hri); RegionStates regionStates = am.getRegionStates(); ServerName serverName = regionStates.getRegionServerOfRegion(hri); // Assert the the region is actually open on the server diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java index de157b76d3a..292ec728a34 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java @@ -271,13 +271,13 @@ public class TestMasterFailover { for (HRegionInfo hri : enabledAndAssignedRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, serverName)); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } for (HRegionInfo hri : disabledAndAssignedRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, serverName)); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } // wait for no more RIT @@ -604,12 +604,12 @@ public class TestMasterFailover { for (HRegionInfo hri : enabledAndAssignedRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, hrs.getServerName())); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } for (HRegionInfo hri : disabledAndAssignedRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, hrs.getServerName())); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } log("Waiting for assignment to finish"); @@ -632,12 +632,12 @@ public class TestMasterFailover { for (HRegionInfo hri : enabledAndOnDeadRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, deadServerName)); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } for (HRegionInfo hri : disabledAndOnDeadRegions) { master.assignmentManager.addPlan(hri.getEncodedName(), new RegionPlan(hri, null, deadServerName)); - master.assignRegion(hri); + master.assignmentManager.assign(hri, true); } // wait for no more RIT @@ -646,7 +646,7 @@ public class TestMasterFailover { master.assignmentManager.waitUntilNoRegionsInTransition(60000); log("Assignment completed"); - // Due to master.assignRegion(hri) could fail to assign a region to a specified RS + // Due to master.assignment.assign(hri) could fail to assign a region to a specified RS // therefore, we need make sure that regions are in the expected RS verifyRegionLocation(hrs, enabledAndAssignedRegions); verifyRegionLocation(hrs, disabledAndAssignedRegions); @@ -1064,7 +1064,7 @@ public class TestMasterFailover { RegionState newState = regionStates.getRegionState(hri); assertTrue(newState.isOpened()); } - + /** * Simple test of master failover. *

@@ -1246,37 +1246,37 @@ public class TestMasterFailover { oldState = new RegionState(hriOffline, State.OFFLINE); newState = new RegionState(hriOffline, State.PENDING_OPEN, newState.getServerName()); stateStore.updateRegionState(HConstants.NO_SEQNUM, newState, oldState); - + HRegionInfo failedClose = new HRegionInfo(offlineTable.getTableName(), null, null); createRegion(failedClose, rootdir, conf, offlineTable); MetaTableAccessor.addRegionToMeta(master.getConnection(), failedClose); - + oldState = new RegionState(failedClose, State.PENDING_CLOSE); newState = new RegionState(failedClose, State.FAILED_CLOSE, newState.getServerName()); stateStore.updateRegionState(HConstants.NO_SEQNUM, newState, oldState); - - + + HRegionInfo failedOpen = new HRegionInfo(offlineTable.getTableName(), null, null); createRegion(failedOpen, rootdir, conf, offlineTable); MetaTableAccessor.addRegionToMeta(master.getConnection(), failedOpen); - + // Simulate a region transitioning to failed open when the region server reports the // transition as FAILED_OPEN oldState = new RegionState(failedOpen, State.PENDING_OPEN); newState = new RegionState(failedOpen, State.FAILED_OPEN, newState.getServerName()); stateStore.updateRegionState(HConstants.NO_SEQNUM, newState, oldState); - + HRegionInfo failedOpenNullServer = new HRegionInfo(offlineTable.getTableName(), null, null); createRegion(failedOpenNullServer, rootdir, conf, offlineTable); MetaTableAccessor.addRegionToMeta(master.getConnection(), failedOpenNullServer); - + // Simulate a region transitioning to failed open when the master couldn't find a plan for // the region oldState = new RegionState(failedOpenNullServer, State.OFFLINE); newState = new RegionState(failedOpenNullServer, State.FAILED_OPEN, null); stateStore.updateRegionState(HConstants.NO_SEQNUM, newState, oldState); - - + + // Stop the master log("Aborting master"); @@ -1303,7 +1303,7 @@ public class TestMasterFailover { assertTrue(regionStates.isRegionOnline(failedClose)); assertTrue(regionStates.isRegionOnline(failedOpenNullServer)); assertTrue(regionStates.isRegionOnline(failedOpen)); - + log("Done with verification, shutting down cluster"); // Done, shutdown the cluster diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java index 9bbf141d08d..eafbeea00bc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java @@ -699,9 +699,7 @@ public class TestHBaseFsck { // Now let's mess it up, by adding a region with a duplicate startkey HRegionInfo hriDupe = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A"), Bytes.toBytes("A2")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriDupe); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriDupe); + TEST_UTIL.assignRegion(hriDupe); ServerName server = regionStates.getRegionServerOfRegion(hriDupe); TEST_UTIL.assertRegionOnServer(hriDupe, server, REGION_ONLINE_TIMEOUT); @@ -875,9 +873,7 @@ public class TestHBaseFsck { HRegionInfo hriDupe = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A"), Bytes.toBytes("B")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriDupe); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriDupe); + TEST_UTIL.assignRegion(hriDupe); ServerName server = regionStates.getRegionServerOfRegion(hriDupe); TEST_UTIL.assertRegionOnServer(hriDupe, server, REGION_ONLINE_TIMEOUT); @@ -925,9 +921,7 @@ public class TestHBaseFsck { // Now let's mess it up, by adding a region with a duplicate startkey HRegionInfo hriDupe = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("B"), Bytes.toBytes("B")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriDupe); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriDupe); + TEST_UTIL.assignRegion(hriDupe); ServerName server = regionStates.getRegionServerOfRegion(hriDupe); TEST_UTIL.assertRegionOnServer(hriDupe, server, REGION_ONLINE_TIMEOUT); @@ -965,9 +959,8 @@ public class TestHBaseFsck { // Mess it up by creating an overlap in the metadata HRegionInfo hriOverlap = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A2"), Bytes.toBytes("B")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriOverlap); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriOverlap); + TEST_UTIL.assignRegion(hriOverlap); + ServerName server = regionStates.getRegionServerOfRegion(hriOverlap); TEST_UTIL.assertRegionOnServer(hriOverlap, server, REGION_ONLINE_TIMEOUT); @@ -1009,12 +1002,10 @@ public class TestHBaseFsck { HMaster master = cluster.getMaster(); HRegionInfo hriOverlap1 = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A"), Bytes.toBytes("AB")); - master.assignRegion(hriOverlap1); - master.getAssignmentManager().waitForAssignment(hriOverlap1); + TEST_UTIL.assignRegion(hriOverlap1); HRegionInfo hriOverlap2 = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("AB"), Bytes.toBytes("B")); - master.assignRegion(hriOverlap2); - master.getAssignmentManager().waitForAssignment(hriOverlap2); + TEST_UTIL.assignRegion(hriOverlap2); HBaseFsck hbck = doFsck(conf, false); assertErrors(hbck, new ERROR_CODE[] {ERROR_CODE.DUPE_STARTKEYS, @@ -1104,9 +1095,7 @@ public class TestHBaseFsck { HRegionInfo hriOverlap = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A2"), Bytes.toBytes("B")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriOverlap); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriOverlap); + TEST_UTIL.assignRegion(hriOverlap); ServerName server = regionStates.getRegionServerOfRegion(hriOverlap); TEST_UTIL.assertRegionOnServer(hriOverlap, server, REGION_ONLINE_TIMEOUT); @@ -1144,9 +1133,7 @@ public class TestHBaseFsck { // Mess it up by creating an overlap in the metadata HRegionInfo hriOverlap = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A2"), Bytes.toBytes("B2")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriOverlap); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriOverlap); + TEST_UTIL.assignRegion(hriOverlap); ServerName server = regionStates.getRegionServerOfRegion(hriOverlap); TEST_UTIL.assertRegionOnServer(hriOverlap, server, REGION_ONLINE_TIMEOUT); @@ -2061,9 +2048,7 @@ public class TestHBaseFsck { HRegionInfo hriOverlap = createRegion(tbl.getTableDescriptor(), Bytes.toBytes("A2"), Bytes.toBytes("B")); - TEST_UTIL.getHBaseCluster().getMaster().assignRegion(hriOverlap); - TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager() - .waitForAssignment(hriOverlap); + TEST_UTIL.assignRegion(hriOverlap); ServerName server = regionStates.getRegionServerOfRegion(hriOverlap); TEST_UTIL.assertRegionOnServer(hriOverlap, server, REGION_ONLINE_TIMEOUT);