HBASE-18025 CatalogJanitor should collect outdated RegionStates from the AM
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/71a9a9a9 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/71a9a9a9 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/71a9a9a9 Branch: refs/heads/HBASE-14070.HLC Commit: 71a9a9a9440c9f2e2e9dd301dd372197e38e70c5 Parents: 043ec9b Author: Esteban Gutierrez <este...@apache.org> Authored: Fri Jul 21 14:13:13 2017 -0500 Committer: Esteban Gutierrez <este...@apache.org> Committed: Fri Aug 11 13:36:38 2017 -0500 ---------------------------------------------------------------------- .../hadoop/hbase/master/CatalogJanitor.java | 13 +- .../hadoop/hbase/master/ServerManager.java | 7 + .../hbase/master/assignment/RegionStates.java | 6 + .../TestCatalogJanitorInMemoryStates.java | 185 +++++++++++++++++++ 4 files changed, 209 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/71a9a9a9/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java index ba92c76..8daa7db 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java @@ -221,6 +221,11 @@ public class CatalogJanitor extends ScheduledChore { ProcedureExecutor<MasterProcedureEnv> pe = this.services.getMasterProcedureExecutor(); pe.submitProcedure(new GCMergedRegionsProcedure(pe.getEnvironment(), mergedRegion, regionA, regionB)); + // Remove from in-memory states + this.services.getAssignmentManager().getRegionStates().deleteRegion(regionA); + this.services.getAssignmentManager().getRegionStates().deleteRegion(regionB); + this.services.getServerManager().removeRegion(regionA); + this.services.getServerManager().removeRegion(regionB); return true; } return false; @@ -234,6 +239,7 @@ public class CatalogJanitor extends ScheduledChore { */ int scan() throws IOException { int result = 0; + try { if (!alreadyRunning.compareAndSet(false, true)) { LOG.debug("CatalogJanitor already running"); @@ -281,8 +287,8 @@ public class CatalogJanitor extends ScheduledChore { } if (!parentNotCleaned.contains(e.getKey().getEncodedName()) && - cleanParent(e.getKey(), e.getValue())) { - result++; + cleanParent(e.getKey(), e.getValue())) { + result++; } else { // We could not clean the parent, so it's daughters should not be // cleaned either (HBASE-6160) @@ -355,6 +361,9 @@ public class CatalogJanitor extends ScheduledChore { " -- no longer hold references"); ProcedureExecutor<MasterProcedureEnv> pe = this.services.getMasterProcedureExecutor(); pe.submitProcedure(new GCRegionProcedure(pe.getEnvironment(), parent)); + // Remove from in-memory states + this.services.getAssignmentManager().getRegionStates().deleteRegion(parent); + this.services.getServerManager().removeRegion(parent); return true; } return false; http://git-wip-us.apache.org/repos/asf/hbase/blob/71a9a9a9/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index c9c792a..f0e9b88 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -1028,6 +1028,13 @@ public class ServerManager { flushedSequenceIdByRegion.remove(encodedName); } + @VisibleForTesting + public boolean isRegionInServerManagerStates(final HRegionInfo hri) { + final byte[] encodedName = hri.getEncodedNameAsBytes(); + return (storeFlushedSequenceIdsByRegion.containsKey(encodedName) + || flushedSequenceIdByRegion.containsKey(encodedName)); + } + /** * Called by delete table and similar to notify the ServerManager that a region was removed. */ http://git-wip-us.apache.org/repos/asf/hbase/blob/71a9a9a9/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java index df55c94..1169dda 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java @@ -432,6 +432,12 @@ public class RegionStates { serverMap.clear(); } + @VisibleForTesting + public boolean isRegionInRegionStates(final HRegionInfo hri) { + return (regionsMap.containsKey(hri.getRegionName()) || regionInTransition.containsKey(hri) + || regionOffline.containsKey(hri)); + } + // ========================================================================== // RegionStateNode helpers // ========================================================================== http://git-wip-us.apache.org/repos/asf/hbase/blob/71a9a9a9/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java new file mode 100644 index 0000000..38abe57 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java @@ -0,0 +1,185 @@ +/** + * + * 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 org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.*; +import org.apache.hadoop.hbase.client.*; +import org.apache.hadoop.hbase.master.assignment.AssignmentManager; +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.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.PairOfSameType; +import org.apache.hadoop.hbase.util.Threads; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.junit.rules.TestName; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; + +@Category({MasterTests.class, MediumTests.class}) +public class TestCatalogJanitorInMemoryStates { + private static final Log LOG = LogFactory.getLog(TestCatalogJanitorInMemoryStates.class); + @Rule public final TestRule timeout = CategoryBasedTimeout.builder(). + withTimeout(this.getClass()).withLookingForStuckThread(true).build(); + @Rule public final TestName name = new TestName(); + protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static byte [] ROW = Bytes.toBytes("testRow"); + private static byte [] FAMILY = Bytes.toBytes("testFamily"); + private static byte [] QUALIFIER = Bytes.toBytes("testQualifier"); + private static byte [] VALUE = Bytes.toBytes("testValue"); + + /** + * @throws java.lang.Exception + */ + @BeforeClass + public static void setUpBeforeClass() throws Exception { + Configuration conf = TEST_UTIL.getConfiguration(); + TEST_UTIL.startMiniCluster(1); + } + + /** + * @throws java.lang.Exception + */ + @AfterClass + public static void tearDownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + /** + * Test clearing a split parent from memory. + */ + @Test(timeout = 180000) + public void testInMemoryParentCleanup() throws IOException, InterruptedException { + final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager(); + final ServerManager sm = TEST_UTIL.getHBaseCluster().getMaster().getServerManager(); + final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().getCatalogJanitor(); + + Admin admin = TEST_UTIL.getAdmin(); + admin.enableCatalogJanitor(false); + + final TableName tableName = TableName.valueOf(name.getMethodName()); + Table t = TEST_UTIL.createTable(tableName, FAMILY); + int rowCount = TEST_UTIL.loadTable(t, FAMILY, false); + + RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName); + List<HRegionLocation> allRegionLocations = locator.getAllRegionLocations(); + + // We need to create a valid split with daughter regions + HRegionLocation parent = allRegionLocations.get(0); + List<HRegionLocation> daughters = splitRegion(parent.getRegionInfo()); + LOG.info("Parent region: " + parent); + LOG.info("Daughter regions: " + daughters); + assertNotNull("Should have found daughter regions for " + parent, daughters); + + assertTrue("Parent region should exist in RegionStates", + am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo())); + assertTrue("Parent region should exist in ServerManager", + sm.isRegionInServerManagerStates(parent.getRegionInfo())); + + // clean the parent + Result r = MetaMockingUtil.getMetaTableRowResult(parent.getRegionInfo(), null, + daughters.get(0).getRegionInfo(), daughters.get(1).getRegionInfo()); + janitor.cleanParent(parent.getRegionInfo(), r); + assertFalse("Parent region should have been removed from RegionStates", + am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo())); + assertFalse("Parent region should have been removed from ServerManager", + sm.isRegionInServerManagerStates(parent.getRegionInfo())); + + } + + /* + * Splits a region + * @param t Region to split. + * @return List of region locations + * @throws IOException, InterruptedException + */ + private List<HRegionLocation> splitRegion(final HRegionInfo r) + throws IOException, InterruptedException { + List<HRegionLocation> locations = new ArrayList<>(); + // Split this table in two. + Admin admin = TEST_UTIL.getAdmin(); + Connection connection = TEST_UTIL.getConnection(); + admin.splitRegion(r.getEncodedNameAsBytes()); + admin.close(); + PairOfSameType<HRegionInfo> regions = waitOnDaughters(r); + if (regions != null) { + try (RegionLocator rl = connection.getRegionLocator(r.getTable())) { + locations.add(rl.getRegionLocation(regions.getFirst().getEncodedNameAsBytes())); + locations.add(rl.getRegionLocation(regions.getSecond().getEncodedNameAsBytes())); + } + return locations; + } + return locations; + } + + /* + * Wait on region split. May return because we waited long enough on the split + * and it didn't happen. Caller should check. + * @param r + * @return Daughter regions; caller needs to check table actually split. + */ + private PairOfSameType<HRegionInfo> waitOnDaughters(final HRegionInfo r) + throws IOException { + long start = System.currentTimeMillis(); + PairOfSameType<HRegionInfo> pair = null; + try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration()); + Table metaTable = conn.getTable(TableName.META_TABLE_NAME)) { + Result result = null; + HRegionInfo region = null; + while ((System.currentTimeMillis() - start) < 60000) { + result = metaTable.get(new Get(r.getRegionName())); + if (result == null) { + break; + } + region = MetaTableAccessor.getHRegionInfo(result); + if (region.isSplitParent()) { + LOG.debug(region.toString() + " IS a parent!"); + pair = MetaTableAccessor.getDaughterRegions(result); + break; + } + Threads.sleep(100); + } + + if (pair.getFirst() == null || pair.getSecond() == null) { + throw new IOException("Failed to get daughters, for parent region: " + r); + } + return pair; + } + } +} \ No newline at end of file
