diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index ae2c993180a..3cb41eb04f8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -172,11 +172,11 @@ public class TransitRegionStateProcedure private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) throws ProcedureSuspendedException { - // Here the assumption is that, the region must be in CLOSED state, so the region location - // will be null. And if we fail to open the region and retry here, the forceNewPlan will be - // true, and also we will set the region location to null. boolean retain = false; - if (!forceNewPlan) { + if (forceNewPlan) { + // set the region location to null if forceNewPlan is true + regionNode.setRegionLocation(null); + } else { if (assignCandidate != null) { retain = assignCandidate.equals(regionNode.getLastHost()); regionNode.setRegionLocation(assignCandidate); @@ -399,6 +399,7 @@ public class TransitRegionStateProcedure public void serverCrashed(MasterProcedureEnv env, RegionStateNode regionNode, ServerName serverName) throws IOException { // force to assign to a new candidate server + // AssignmentManager#regionClosedAbnormally will set region location to null // TODO: the forceNewPlan flag not be persistent so if master crash then the flag will be lost. // But assign to old server is not big deal because it not effect correctness. // See HBASE-23035 for more details. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java index 062490e687e..fbf128f0f40 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Collection; import java.util.EnumSet; import java.util.HashMap; @@ -33,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.ClusterMetrics.Option; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -43,7 +43,6 @@ import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.MetaTableAccessor.Visitor; import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.StartMiniClusterOption; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; @@ -59,7 +58,6 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.JVMClusterUtil; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -194,24 +192,6 @@ public class TestMasterOperationsForRegionReplicas { assertNotNull(state); } } - validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, numReplica, - ADMIN.getConnection()); - // Now shut the whole cluster down, and verify the assignments are kept so that the - // availability constraints are met. MiniHBaseCluster chooses arbitrary ports on each - // restart. This messes with our being able to test that we retain locality. Therefore, - // figure current cluster ports and pass them in on next cluster start so new cluster comes - // up at same coordinates -- and the assignment retention logic has a chance to cut in. - List rsports = new ArrayList<>(); - for (JVMClusterUtil.RegionServerThread rst : TEST_UTIL.getHBaseCluster() - .getLiveRegionServerThreads()) { - rsports.add(rst.getRegionServer().getRpcServer().getListenerAddress().getPort()); - } - TEST_UTIL.shutdownMiniHBaseCluster(); - StartMiniClusterOption option = - StartMiniClusterOption.builder().numRegionServers(numSlaves).rsPorts(rsports).build(); - TEST_UTIL.startMiniHBaseCluster(option); - TEST_UTIL.waitUntilAllRegionsAssigned(tableName); - TEST_UTIL.waitUntilNoRegionsInTransition(); validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, numReplica, ADMIN.getConnection()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java deleted file mode 100644 index 7134e0ee8f5..00000000000 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java +++ /dev/null @@ -1,147 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.master; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; - -import java.util.List; -import java.util.Map; -import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.MiniHBaseCluster; -import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.TableName; -import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.testclassification.MasterTests; -import org.apache.hadoop.hbase.testclassification.MediumTests; -import org.apache.hadoop.hbase.util.JVMClusterUtil; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Category({ MasterTests.class, MediumTests.class }) -public class TestRetainAssignmentOnRestart extends AbstractTestRestartCluster { - - @ClassRule - public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestRetainAssignmentOnRestart.class); - - private static final Logger LOG = LoggerFactory.getLogger(TestRetainAssignmentOnRestart.class); - - @Override - protected boolean splitWALCoordinatedByZk() { - return true; - } - - /** - * This tests retaining assignments on a cluster restart - */ - @Test - public void test() throws Exception { - UTIL.startMiniCluster(2); - // Turn off balancer - UTIL.getMiniHBaseCluster().getMaster().getMasterRpcServices().synchronousBalanceSwitch(false); - LOG.info("\n\nCreating tables"); - for (TableName TABLE : TABLES) { - UTIL.createTable(TABLE, FAMILY); - } - for (TableName TABLE : TABLES) { - UTIL.waitTableEnabled(TABLE); - } - - HMaster master = UTIL.getMiniHBaseCluster().getMaster(); - UTIL.waitUntilNoRegionsInTransition(60000); - - // We don't have to use SnapshotOfRegionAssignmentFromMeta. - // We use it here because AM used to use it to load all user region placements - SnapshotOfRegionAssignmentFromMeta snapshot = - new SnapshotOfRegionAssignmentFromMeta(master.getConnection()); - snapshot.initialize(); - Map regionToRegionServerMap = snapshot.getRegionToRegionServerMap(); - - MiniHBaseCluster cluster = UTIL.getHBaseCluster(); - List threads = cluster.getLiveRegionServerThreads(); - assertEquals(2, threads.size()); - int[] rsPorts = new int[3]; - for (int i = 0; i < 2; i++) { - rsPorts[i] = threads.get(i).getRegionServer().getServerName().getPort(); - } - rsPorts[2] = cluster.getMaster().getServerName().getPort(); - for (ServerName serverName : regionToRegionServerMap.values()) { - boolean found = false; // Test only, no need to optimize - for (int k = 0; k < 3 && !found; k++) { - found = serverName.getPort() == rsPorts[k]; - } - assertTrue(found); - } - - LOG.info("\n\nShutting down HBase cluster"); - cluster.stopMaster(0); - cluster.shutdown(); - cluster.waitUntilShutDown(); - - LOG.info("\n\nSleeping a bit"); - Thread.sleep(2000); - - LOG.info("\n\nStarting cluster the second time with the same ports"); - cluster.getConf().setInt(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]); - cluster.startRegionServer(); - } - - // Make sure live regionservers are on the same host/port - List localServers = master.getServerManager().getOnlineServersList(); - assertEquals(3, localServers.size()); - for (int i = 0; i < 3; i++) { - boolean found = false; - for (ServerName serverName : localServers) { - if (serverName.getPort() == rsPorts[i]) { - found = true; - break; - } - } - assertTrue(found); - } - - // Wait till master is initialized and all regions are assigned - for (TableName TABLE : TABLES) { - UTIL.waitTableAvailable(TABLE); - } - UTIL.waitUntilNoRegionsInTransition(60000); - - snapshot = new SnapshotOfRegionAssignmentFromMeta(master.getConnection()); - snapshot.initialize(); - Map newRegionToRegionServerMap = snapshot.getRegionToRegionServerMap(); - assertEquals(regionToRegionServerMap.size(), newRegionToRegionServerMap.size()); - for (Map.Entry entry : newRegionToRegionServerMap.entrySet()) { - ServerName oldServer = regionToRegionServerMap.get(entry.getKey()); - ServerName currentServer = entry.getValue(); - LOG.info( - "Key=" + entry.getKey() + " oldServer=" + oldServer + ", currentServer=" + currentServer); - assertEquals(entry.getKey().toString(), oldServer.getAddress(), currentServer.getAddress()); - assertNotEquals(oldServer.getStartcode(), currentServer.getStartcode()); - } - } - -} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestart.java new file mode 100644 index 00000000000..4acb7891154 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestart.java @@ -0,0 +1,117 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.List; + +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MiniHBaseCluster; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.testclassification.MasterTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.JVMClusterUtil; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Category({ MasterTests.class, MediumTests.class }) +public class TestRoundRobinAssignmentOnRestart extends AbstractTestRestartCluster { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestRoundRobinAssignmentOnRestart.class); + + private static final Logger LOG = + LoggerFactory.getLogger(TestRoundRobinAssignmentOnRestart.class); + + @Override + protected boolean splitWALCoordinatedByZk() { + return true; + } + + private final int regionNum = 10; + private final int rsNum = 2; + + /** + * This tests retaining assignments on a cluster restart + */ + @Test + public void test() throws Exception { + UTIL.startMiniCluster(rsNum); + // Turn off balancer + UTIL.getMiniHBaseCluster().getMaster().getMasterRpcServices().synchronousBalanceSwitch(false); + LOG.info("\n\nCreating tables"); + for (TableName TABLE : TABLES) { + UTIL.createMultiRegionTable(TABLE, FAMILY, regionNum); + } + // Wait until all regions are assigned + for (TableName TABLE : TABLES) { + UTIL.waitTableEnabled(TABLE); + } + UTIL.waitUntilNoRegionsInTransition(60000); + + MiniHBaseCluster cluster = UTIL.getHBaseCluster(); + List threads = cluster.getLiveRegionServerThreads(); + assertEquals(2, threads.size()); + + ServerName testServer = threads.get(0).getRegionServer().getServerName(); + int port = testServer.getPort(); + List regionInfos = + cluster.getMaster().getAssignmentManager().getRegionsOnServer(testServer); + LOG.debug("RegionServer {} has {} regions", testServer, regionInfos.size()); + assertTrue(regionInfos.size() >= (TABLES.length * regionNum / rsNum)); + + // Restart 1 regionserver + cluster.stopRegionServer(testServer); + cluster.waitForRegionServerToStop(testServer, 60000); + cluster.getConf().setInt(HConstants.REGIONSERVER_PORT, port); + cluster.startRegionServer(); + + HMaster master = UTIL.getMiniHBaseCluster().getMaster(); + List localServers = master.getServerManager().getOnlineServersList(); + ServerName newTestServer = null; + for (ServerName serverName : localServers) { + if (serverName.getAddress().equals(testServer.getAddress())) { + newTestServer = serverName; + break; + } + } + assertNotNull(newTestServer); + + // Wait until all regions are assigned + for (TableName TABLE : TABLES) { + UTIL.waitTableAvailable(TABLE); + } + UTIL.waitUntilNoRegionsInTransition(60000); + + List newRegionInfos = + cluster.getMaster().getAssignmentManager().getRegionsOnServer(newTestServer); + LOG.debug("RegionServer {} has {} regions", newTestServer, newRegionInfos.size()); + assertTrue("Should not retain all regions when restart", + newRegionInfos.size() < regionInfos.size()); + } +} \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestartSplitWithoutZk.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestartSplitWithoutZk.java similarity index 86% rename from hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestartSplitWithoutZk.java rename to hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestartSplitWithoutZk.java index faf2679c448..d16f36f9e57 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestartSplitWithoutZk.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRoundRobinAssignmentOnRestartSplitWithoutZk.java @@ -24,11 +24,12 @@ import org.junit.ClassRule; import org.junit.experimental.categories.Category; @Category({ MasterTests.class, MediumTests.class }) -public class TestRetainAssignmentOnRestartSplitWithoutZk extends TestRetainAssignmentOnRestart { +public class TestRoundRobinAssignmentOnRestartSplitWithoutZk + extends TestRoundRobinAssignmentOnRestart { @ClassRule public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestRetainAssignmentOnRestartSplitWithoutZk.class); + HBaseClassTestRule.forClass(TestRoundRobinAssignmentOnRestartSplitWithoutZk.class); @Override protected boolean splitWALCoordinatedByZk() { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPBase.java index b3bfd2f83e0..fa3d28c3367 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPBase.java @@ -19,11 +19,9 @@ package org.apache.hadoop.hbase.master.procedure; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; @@ -31,14 +29,11 @@ import org.apache.hadoop.hbase.MiniHBaseCluster; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.assignment.AssignmentTestingUtil; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; -import org.apache.hadoop.hbase.regionserver.Region; -import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread; import org.junit.After; import org.junit.Before; import org.slf4j.Logger; @@ -130,7 +125,6 @@ public class TestSCPBase { long procId = getSCPProcId(procExec); ProcedureTestingUtility.waitProcedure(procExec, procId); } - assertReplicaDistributed(t); assertEquals(count, HBaseTestingUtility.countRows(t)); assertEquals(checksum, util.checksumRows(t)); } @@ -141,36 +135,6 @@ public class TestSCPBase { return procExec.getActiveProcIds().stream().mapToLong(Long::longValue).min().getAsLong(); } - private void assertReplicaDistributed(Table t) throws IOException { - if (t.getDescriptor().getRegionReplication() <= 1) { - return; - } - // Assert all data came back. - List regionInfos = new ArrayList<>(); - for (RegionServerThread rs : this.util.getMiniHBaseCluster().getRegionServerThreads()) { - regionInfos.clear(); - for (Region r : rs.getRegionServer().getRegions(t.getName())) { - LOG.info("The region is " + r.getRegionInfo() + " the location is " + - rs.getRegionServer().getServerName()); - if (contains(regionInfos, r.getRegionInfo())) { - LOG.error("Am exiting"); - fail("Crashed replica regions should not be assigned to same region server"); - } else { - regionInfos.add(r.getRegionInfo()); - } - } - } - } - - private boolean contains(List regionInfos, RegionInfo regionInfo) { - for (RegionInfo info : regionInfos) { - if (RegionReplicaUtil.isReplicasForSameRegion(info, regionInfo)) { - return true; - } - } - return false; - } - protected Table createTable(final TableName tableName) throws IOException { final Table t = this.util.createTable(tableName, HBaseTestingUtility.COLUMNS, HBaseTestingUtility.KEYS_FOR_HBA_CREATE_TABLE, getRegionReplication()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java index a4c09188ee2..2458a7d5a18 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java @@ -75,7 +75,7 @@ public class TestRegionReplicasWithRestartScenarios { HTU.getConfiguration().setInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, 8192); HTU.getConfiguration().setInt(DFSConfigKeys.DFS_CLIENT_READ_PREFETCH_SIZE_KEY, 1); HTU.getConfiguration().setInt(HConstants.HREGION_MEMSTORE_FLUSH_SIZE, 128 * 1024 * 1024); - HTU.getConfiguration().setInt(">hbase.master.wait.on.regionservers.mintostart", 3); + HTU.getConfiguration().setInt("hbase.master.wait.on.regionservers.mintostart", 3); HTU.startMiniCluster(NB_SERVERS); }