diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index 5baf30846e08..1a87aa2360ff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -1862,27 +1862,33 @@ public void visitRegionState(Result result, final RegionInfo regionInfo, final S LOG.info(regionInfo.getEncodedName() + " regionState=null; presuming " + State.OFFLINE); localState = State.OFFLINE; } + // hbase:meta keeps stale info:sn / info:server after a region transitions to a non-online + // state, so retain regionLocation only for states that have "current host" semantics. The + // set below mirrors the one that drives addRegionToServer(regionNode), preserving the + // invariant: non-null regionLocation iff member of a ServerStateNode. + boolean hasCurrentHostSemantics = localState.matches(State.OPEN, State.OPENING, + State.CLOSING, State.SPLITTING, State.MERGING); + ServerName normalizedLocation = hasCurrentHostSemantics ? regionLocation : null; + RegionStateNode regionNode = regionStates.getOrCreateRegionStateNode(regionInfo); // Do not need to lock on regionNode, as we can make sure that before we finish loading // meta, all the related procedures can not be executed. The only exception is for meta // region related operations, but here we do not load the informations for meta region. regionNode.setState(localState); regionNode.setLastHost(lastHost); - regionNode.setRegionLocation(regionLocation); + regionNode.setRegionLocation(normalizedLocation); regionNode.setOpenSeqNum(openSeqNum); // Note: keep consistent with other methods, see region(Opening|Opened|Closing) - // RIT/ServerCrash handling should take care of the transiting regions. - if ( - localState.matches(State.OPEN, State.OPENING, State.CLOSING, State.SPLITTING, State.MERGING) - ) { - assert regionLocation != null : "found null region location for " + regionNode; + // RIT/ServerCrash handling should take care of the transiting regions. + if (hasCurrentHostSemantics) { + assert normalizedLocation != null : "found null region location for " + regionNode; // TODO: this could lead to some orphan server state nodes, as it is possible that the // region server is already dead and its SCP has already finished but we have // persisted an opening state on this region server. Finally the TRSP will assign the // region to another region server, so it will not cause critical problems, just waste // some memory as no one will try to cleanup these orphan server state nodes. - regionStates.createServer(regionLocation); + regionStates.createServer(normalizedLocation); regionStates.addRegionToServer(regionNode); } else if (localState == State.OFFLINE || regionInfo.isOffline()) { regionStates.addToOfflineRegions(regionNode); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerLoadMetaRegionState.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerLoadMetaRegionState.java index 295f99dc9031..a7346a0e16dc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerLoadMetaRegionState.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerLoadMetaRegionState.java @@ -18,18 +18,33 @@ package org.apache.hadoop.hbase.master.assignment; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import org.apache.hadoop.hbase.CatalogFamilyFormat; import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.RegionState.State; +import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; @@ -40,6 +55,7 @@ public class TestAssignmentManagerLoadMetaRegionState { private static final HBaseTestingUtil UTIL = new HBaseTestingUtil(); + private static final byte[] CF = Bytes.toBytes("cf"); @BeforeAll public static void setUp() throws Exception { @@ -60,6 +76,7 @@ public void testRestart() throws InterruptedException, IOException { UTIL.getMiniHBaseCluster().stopMaster(0).join(); HMaster newMaster = UTIL.getMiniHBaseCluster().startMaster().getMaster(); UTIL.waitFor(30000, () -> newMaster.isInitialized()); + UTIL.invalidateConnection(); am = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager(); List newRegions = am.getRegionsOnServer(sn); @@ -68,4 +85,176 @@ public void testRestart() throws InterruptedException, IOException { assertTrue(regions.contains(region)); } } + + @Test + public void testRestartWithClosedRegion() throws Exception { + final TableName tableName = TableName.valueOf("testRestartWithClosedRegion"); + UTIL.createTable(tableName, CF); + try (Admin admin = UTIL.getConnection().getAdmin()) { + List regions = UTIL.getHBaseCluster().getRegions(tableName); + assertEquals(1, regions.size()); + RegionInfo region = regions.get(0).getRegionInfo(); + ServerName hostBeforeClose = UTIL.getHBaseCluster().getMaster().getAssignmentManager() + .getRegionStates().getRegionServerOfRegion(region); + assertNotNull(hostBeforeClose, "region must be assigned before disable"); + + admin.disableTable(tableName); + + AssignmentManager masterBeforeRestart = + UTIL.getHBaseCluster().getMaster().getAssignmentManager(); + RegionStateNode nodeBeforeRestart = + masterBeforeRestart.getRegionStates().getRegionStateNode(region); + assertEquals(State.CLOSED, nodeBeforeRestart.getState(), + "region should be CLOSED after disable"); + assertNull(nodeBeforeRestart.getRegionLocation(), + "regionLocation should be null for a CLOSED region before master failover"); + + restartActiveMaster(); + + AssignmentManager masterAfterRestart = + UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager(); + RegionStateNode nodeAfterRestart = + masterAfterRestart.getRegionStates().getRegionStateNode(region); + assertNotNull(nodeAfterRestart, "region state node must be restored from meta"); + assertEquals(State.CLOSED, nodeAfterRestart.getState(), + "state should still be CLOSED after failover"); + assertNull(nodeAfterRestart.getRegionLocation(), + "regionLocation must be null for a CLOSED region after master failover " + + "(meta keeps stale info:sn, but in-memory state must be normalized)"); + assertEquals(hostBeforeClose, nodeAfterRestart.getLastHost(), + "lastHost must be preserved across failover for locality / recovery"); + assertEquals(hostBeforeClose, + masterAfterRestart.getRegionStates().getRegionServerOfRegion(region), + "closed region should still resolve to lastHost for locality-sensitive paths"); + assertNull(masterAfterRestart.getRegionStates().getRegionAssignments().get(region), + "getRegionAssignments must not export a stale current owner for CLOSED region"); + Map> assignmentSnapshot = + masterAfterRestart.getSnapShotOfAssignment(Collections.singleton(region)); + assertFalse(assignmentSnapshot.containsKey(hostBeforeClose), + "getSnapShotOfAssignment must not place CLOSED region under a live server"); + List regionsOnHost = masterAfterRestart.getRegionsOnServer(hostBeforeClose); + assertFalse(regionsOnHost.contains(region), + "CLOSED region must not appear in getRegionsOnServer after failover " + + "(ServerStateNode membership must stay consistent with regionLocation == null)"); + + try (Admin postRestartAdmin = UTIL.getAdmin()) { + postRestartAdmin.deleteTable(tableName); + } + } + } + + @Test + public void testRestartWithOpenRegion() throws Exception { + final TableName tableName = TableName.valueOf("testRestartWithOpenRegion"); + UTIL.createTable(tableName, CF); + try (Admin admin = UTIL.getConnection().getAdmin()) { + List regions = UTIL.getHBaseCluster().getRegions(tableName); + assertEquals(1, regions.size()); + RegionInfo region = regions.get(0).getRegionInfo(); + ServerName host = UTIL.getHBaseCluster().getMaster().getAssignmentManager() + .getRegionStates().getRegionServerOfRegion(region); + assertNotNull(host, "region must be assigned before failover"); + + restartActiveMaster(); + + AssignmentManager am = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager(); + RegionStateNode node = am.getRegionStates().getRegionStateNode(region); + assertNotNull(node, "region state node must be restored from meta"); + assertEquals(State.OPEN, node.getState(), "state should remain OPEN after failover"); + assertEquals(host, node.getRegionLocation(), + "regionLocation must match the hosting RS after failover"); + assertEquals(host, am.getRegionStates().getRegionAssignments().get(region), + "getRegionAssignments must keep exporting current owner for OPEN region"); + Map> assignmentSnapshot = + am.getSnapShotOfAssignment(Collections.singleton(region)); + assertTrue(assignmentSnapshot.get(host).contains(region), + "getSnapShotOfAssignment must keep OPEN region under its hosting RS"); + List regionsOnHost = am.getRegionsOnServer(host); + assertTrue(regionsOnHost.contains(region), + "OPEN region must appear in getRegionsOnServer after failover"); + + try (Admin postRestartAdmin = UTIL.getAdmin()) { + postRestartAdmin.disableTable(tableName); + postRestartAdmin.deleteTable(tableName); + } + } + } + + @Test + public void testRestartWithImplicitOfflineRegionCanBeAssigned() throws Exception { + TableName tableName = TableName.valueOf("testRestartWithImplicitOfflineRegionCanBeAssigned"); + UTIL.createTable(tableName, CF); + try (Admin admin = UTIL.getConnection().getAdmin()) { + List regions = UTIL.getHBaseCluster().getRegions(tableName); + assertEquals(1, regions.size()); + RegionInfo region = regions.get(0).getRegionInfo(); + ServerName hostBeforeDisable = UTIL.getHBaseCluster().getMaster().getAssignmentManager() + .getRegionStates().getRegionServerOfRegion(region); + assertNotNull(hostBeforeDisable, "region must be assigned before disable"); + + admin.disableTable(tableName); + setMetaLocation(region, hostBeforeDisable); + + restartActiveMaster(); + + AssignmentManager am = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager(); + RegionStateNode nodeAfterRestart = am.getRegionStates().getRegionStateNode(region); + assertNotNull(nodeAfterRestart, "region state node must be restored from meta"); + assertEquals(State.OFFLINE, nodeAfterRestart.getState(), + "missing state in meta should be restored as OFFLINE"); + assertNull(nodeAfterRestart.getRegionLocation(), + "OFFLINE region must not restore stale regionLocation after failover"); + assertEquals(hostBeforeDisable, nodeAfterRestart.getLastHost(), + "lastHost must be preserved across failover"); + assertTrue(am.getRegionStates().isRegionOffline(region), + "OFFLINE region must still be treated as offline after failover"); + + try (Admin postRestartAdmin = UTIL.getAdmin()) { + postRestartAdmin.enableTable(tableName); + UTIL.waitFor(30000, () -> { + RegionStateNode node = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager() + .getRegionStates().getRegionStateNode(region); + return node != null && node.isInState(State.OPEN) && node.getRegionLocation() != null; + }); + + RegionStateNode nodeAfterEnable = UTIL.getMiniHBaseCluster().getMaster() + .getAssignmentManager().getRegionStates().getRegionStateNode(region); + assertEquals(State.OPEN, nodeAfterEnable.getState(), + "OFFLINE region should be assignable after failover"); + assertNotNull(nodeAfterEnable.getRegionLocation(), + "assigned region must have a live location"); + + postRestartAdmin.disableTable(tableName); + postRestartAdmin.deleteTable(tableName); + } + } + } + + private void setMetaLocation(RegionInfo region, ServerName staleLocation) + throws IOException { + Delete delete = new Delete(region.getRegionName()); + delete.addColumns(HConstants.CATALOG_FAMILY, + CatalogFamilyFormat.getRegionStateColumn(region.getReplicaId())); + Put put = new Put(region.getRegionName()); + put.addColumn(HConstants.CATALOG_FAMILY, + CatalogFamilyFormat.getServerNameColumn(region.getReplicaId()), + Bytes.toBytes(staleLocation.getServerName())); + try (Table meta = UTIL.getConnection().getTable(TableName.META_TABLE_NAME)) { + meta.delete(delete); + meta.put(put); + } + } + + private void restartActiveMaster() throws Exception { + ServerName oldMasterSn = UTIL.getMiniHBaseCluster().getMaster().getServerName(); + UTIL.getMiniHBaseCluster().stopMaster(oldMasterSn); + UTIL.getMiniHBaseCluster().waitForMasterToStop(oldMasterSn, 60000); + UTIL.getMiniHBaseCluster().startMaster(); + UTIL.getMiniHBaseCluster().waitForActiveAndReadyMaster(60000); + UTIL.waitFor(30000, () -> { + HMaster m = UTIL.getMiniHBaseCluster().getMaster(); + return m != null && m.isInitialized(); + }); + UTIL.invalidateConnection(); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMasterAbortWhileSplittingRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMasterAbortWhileSplittingRegion.java new file mode 100644 index 000000000000..cf0f0c363258 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMasterAbortWhileSplittingRegion.java @@ -0,0 +1,126 @@ +/* + * 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.assignment; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CountDownLatch; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.coprocessor.MasterCoprocessor; +import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.MasterObserver; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.testclassification.MasterTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Tag(MasterTests.TAG) +@Tag(MediumTests.TAG) +public class TestMasterAbortWhileSplittingRegion { + + private static final Logger LOG = + LoggerFactory.getLogger(TestMasterAbortWhileSplittingRegion.class); + + private static final HBaseTestingUtil UTIL = new HBaseTestingUtil(); + + private static final TableName TABLE_NAME = TableName.valueOf("testSplit"); + + private static final byte[] CF = Bytes.toBytes("cf"); + + private static final byte[] SPLIT_KEY = Bytes.toBytes("row5"); + + private static final CountDownLatch SPLIT_META_UPDATED = new CountDownLatch(1); + + @BeforeAll + public static void setupCluster() throws Exception { + UTIL.getConfiguration().set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, + SplitRegionObserver.class.getName()); + UTIL.startMiniCluster(3); + UTIL.createTable(TABLE_NAME, CF); + UTIL.waitTableAvailable(TABLE_NAME); + } + + @AfterAll + public static void cleanupTest() throws Exception { + try { + UTIL.shutdownMiniCluster(); + } catch (Exception e) { + LOG.warn("failure shutting down cluster", e); + } + } + + @Test + public void test() throws Exception { + try (Admin admin = UTIL.getAdmin(); + Table table = UTIL.getConnection().getTable(TABLE_NAME)) { + for (int i = 0; i < 10; i++) { + table.put(new Put(Bytes.toBytes("row" + i)).addColumn(CF, CF, CF)); + } + List regionInfos = admin.getRegions(TABLE_NAME); + SplitTableRegionProcedure splitProcedure = new SplitTableRegionProcedure( + UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().getEnvironment(), + regionInfos.get(0), SPLIT_KEY); + long procId = UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor() + .submitProcedure(splitProcedure); + SPLIT_META_UPDATED.await(); + UTIL.getMiniHBaseCluster().stopMaster(0); + UTIL.getMiniHBaseCluster().startMaster(); + UTIL.waitFor(30000, + () -> UTIL.getMiniHBaseCluster().getMaster() != null + && UTIL.getMiniHBaseCluster().getMaster().isInitialized()); + UTIL.waitFor(30000, () -> UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor() + .isFinished(procId)); + assertTrue(UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager() + .getRegionsInTransition().isEmpty(), "Found region RIT, that's impossible! " + + UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager().getRegionsInTransition()); + assertEquals(10, UTIL.countRows(TABLE_NAME), + "Split should keep all rows after master failover"); + assertEquals(2, admin.getRegions(TABLE_NAME).size(), + "Split should leave two daughter regions online"); + } + } + + public static class SplitRegionObserver implements MasterCoprocessor, MasterObserver { + + @Override + public Optional getMasterObserver() { + return Optional.of(this); + } + + @Override + public void preSplitRegionAfterMETAAction( + ObserverContext ctx) { + SPLIT_META_UPDATED.countDown(); + } + } +}