This is an automated email from the ASF dual-hosted git repository. zhangduo pushed a commit to branch HBASE-24950 in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 4967e49cefed7124691b2e1e53fd02e3f3240c8e Author: Duo Zhang <[email protected]> AuthorDate: Fri Sep 11 13:31:00 2020 +0800 HBASE-24607 Implement CatalogJanitor for 'root table' (#2377) Signed-off-by: Guanghao Zhang <[email protected]> --- .../org/apache/hadoop/hbase/MetaTableAccessor.java | 10 --- .../hadoop/hbase/ClientMetaTableAccessor.java | 7 -- .../hbase/master/assignment/GCRegionProcedure.java | 8 +-- .../hbase/master/assignment/RegionStateStore.java | 78 ++++++++++++++++++---- .../hbase/master/janitor/CatalogJanitor.java | 4 +- .../hbase/master/janitor/ReportMakingVisitor.java | 15 ++--- .../hadoop/hbase/TestSimpleMetaSplitMerge.java | 42 +++++++++--- 7 files changed, 105 insertions(+), 59 deletions(-) diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index f171e38..26d8b98 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -19,7 +19,6 @@ package org.apache.hadoop.hbase; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; -import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -49,7 +48,6 @@ import org.apache.hadoop.hbase.filter.SubstringComparator; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.ExceptionUtil; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.PairOfSameType; import org.apache.yetus.audience.InterfaceAudience; @@ -510,14 +508,6 @@ public final class MetaTableAccessor { } } } - if (visitor instanceof Closeable) { - try { - ((Closeable) visitor).close(); - } catch (Throwable t) { - ExceptionUtil.rethrowIfInterrupt(t); - LOG.debug("Got exception in closing the meta scanner visitor", t); - } - } } /** diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ClientMetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ClientMetaTableAccessor.java index 74d2322..ed0d9b4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ClientMetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ClientMetaTableAccessor.java @@ -19,7 +19,6 @@ package org.apache.hadoop.hbase; import static org.apache.hadoop.hbase.util.FutureUtils.addListener; -import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -334,12 +333,6 @@ public final class ClientMetaTableAccessor { } /** - * Implementations 'visit' a catalog table row but with close() at the end. - */ - public interface CloseableVisitor extends Visitor, Closeable { - } - - /** * A {@link Visitor} that collects content out of passed {@link Result}. */ private static abstract class CollectingVisitor<T> implements Visitor { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCRegionProcedure.java index dfd2314..e21e51d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCRegionProcedure.java @@ -109,12 +109,8 @@ public class GCRegionProcedure extends AbstractStateMachineRegionProcedure<GCReg // TODO: Purge metadata before removing from HDFS? This ordering is copied // from CatalogJanitor. AssignmentManager am = masterServices.getAssignmentManager(); - if (am != null) { - if (am.getRegionStates() != null) { - am.getRegionStates().deleteRegion(getRegion()); - } - } - env.getAssignmentManager().getRegionStateStore().deleteRegion(getRegion()); + am.getRegionStates().deleteRegion(getRegion()); + am.getRegionStateStore().deleteRegion(getRegion()); masterServices.getServerManager().removeRegion(getRegion()); FavoredNodesManager fnm = masterServices.getFavoredNodesManager(); if (fnm != null) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java index 117ce89..4e7d271 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java @@ -48,6 +48,7 @@ import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.master.MasterFileSystem; @@ -57,6 +58,7 @@ import org.apache.hadoop.hbase.master.RegionState.State; import org.apache.hadoop.hbase.master.region.MasterRegion; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.util.StringUtils; +import org.apache.hadoop.hbase.regionserver.RegionScanner; import org.apache.hadoop.hbase.replication.ReplicationBarrierFamilyFormat; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -232,6 +234,28 @@ public class RegionStateStore { } } + public void scanCatalog(ClientMetaTableAccessor.Visitor visitor) throws IOException { + // scan meta first + MetaTableAccessor.fullScanRegions(master.getConnection(), visitor); + // scan root + try (RegionScanner scanner = + masterRegion.getScanner(new Scan().addFamily(HConstants.CATALOG_FAMILY))) { + boolean moreRows; + List<Cell> cells = new ArrayList<>(); + do { + moreRows = scanner.next(cells); + if (cells.isEmpty()) { + continue; + } + Result result = Result.create(cells); + cells.clear(); + if (!visitor.visit(result)) { + break; + } + } while (moreRows); + } + } + public void mirrorMetaLocation(RegionInfo regionInfo, ServerName serverName, State state) throws IOException { try { @@ -460,7 +484,7 @@ public class RegionStateStore { if (cells == null || cells.length == 0) { return; } - Delete delete = new Delete(mergeRegion.getRegionName()); + Delete delete = new Delete(CatalogFamilyFormat.getMetaKeyForRegion(mergeRegion)); List<byte[]> qualifiers = new ArrayList<>(); for (Cell cell : cells) { if (!CatalogFamilyFormat.isMergeQualifierPrefix(cell)) { @@ -479,8 +503,13 @@ public class RegionStateStore { " in meta table, they are cleaned up already, Skip."); return; } - try (Table table = master.getConnection().getTable(TableName.META_TABLE_NAME)) { - table.delete(delete); + debugLogMutation(delete); + if (mergeRegion.isMetaRegion()) { + masterRegion.update(r -> r.delete(delete)); + } else { + try (Table table = getMetaTable()) { + table.delete(delete); + } } LOG.info("Deleted merge references in " + mergeRegion.getRegionNameAsString() + ", deleted qualifiers " + @@ -524,19 +553,44 @@ public class RegionStateStore { deleteRegions(regions, EnvironmentEdgeManager.currentTime()); } + private static Delete makeDeleteRegionInfo(RegionInfo regionInfo, long ts) { + return new Delete(CatalogFamilyFormat.getMetaKeyForRegion(regionInfo)) + .addFamily(HConstants.CATALOG_FAMILY, ts); + } + + private static List<Delete> makeDeleteRegionInfos(List<RegionInfo> regionInfos, long ts) { + return regionInfos.stream().map(ri -> makeDeleteRegionInfo(ri, ts)) + .collect(Collectors.toList()); + } + private void deleteRegions(List<RegionInfo> regions, long ts) throws IOException { - List<Delete> deletes = new ArrayList<>(regions.size()); - for (RegionInfo hri : regions) { - Delete e = new Delete(hri.getRegionName()); - e.addFamily(HConstants.CATALOG_FAMILY, ts); - deletes.add(e); + List<RegionInfo> metaRegions = new ArrayList<>(); + List<RegionInfo> nonMetaRegions = new ArrayList<>(); + for (RegionInfo region : regions) { + if (region.isMetaRegion()) { + metaRegions.add(region); + } else { + nonMetaRegions.add(region); + } } - try (Table table = getMetaTable()) { + if (!metaRegions.isEmpty()) { + List<Delete> deletes = makeDeleteRegionInfos(metaRegions, ts); debugLogMutations(deletes); - table.delete(deletes); + for (Delete d : deletes) { + masterRegion.update(r -> r.delete(d)); + } + LOG.info("Deleted {} regions from ROOT", metaRegions.size()); + LOG.debug("Deleted regions: {}", metaRegions); + } + if (!nonMetaRegions.isEmpty()) { + List<Delete> deletes = makeDeleteRegionInfos(nonMetaRegions, ts); + debugLogMutations(deletes); + try (Table table = getMetaTable()) { + table.delete(deletes); + } + LOG.info("Deleted {} regions from META", nonMetaRegions.size()); + LOG.debug("Deleted regions: {}", nonMetaRegions); } - LOG.info("Deleted {} regions from META", regions.size()); - LOG.debug("Deleted regions: {}", regions); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java index 6123f6b..fc25e9b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java @@ -221,8 +221,8 @@ public class CatalogJanitor extends ScheduledChore { @VisibleForTesting protected Report scanForReport() throws IOException { ReportMakingVisitor visitor = new ReportMakingVisitor(this.services); - // Null tablename means scan all of meta. - MetaTableAccessor.scanMetaForTableRegions(this.services.getConnection(), visitor, null); + services.getAssignmentManager().getRegionStateStore().scanCatalog(visitor); + visitor.done(); return visitor.getReport(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java index 4dd514e..77188de 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/ReportMakingVisitor.java @@ -44,7 +44,7 @@ import org.slf4j.LoggerFactory; * {@link #close()}'d. */ @InterfaceAudience.Private -class ReportMakingVisitor implements ClientMetaTableAccessor.CloseableVisitor { +class ReportMakingVisitor implements ClientMetaTableAccessor.Visitor { private static final Logger LOG = LoggerFactory.getLogger(ReportMakingVisitor.class); @@ -199,14 +199,8 @@ class ReportMakingVisitor implements ClientMetaTableAccessor.CloseableVisitor { /** * @return True if table is disabled or disabling; defaults false! */ - boolean isTableDisabled(RegionInfo ri) { - if (ri == null) { - return false; - } - if (this.services == null) { - return false; - } - if (this.services.getTableStateManager() == null) { + private boolean isTableDisabled(RegionInfo ri) { + if (ri.isMetaRegion()) { return false; } TableState state = null; @@ -282,8 +276,7 @@ class ReportMakingVisitor implements ClientMetaTableAccessor.CloseableVisitor { return this.previous == null || !this.previous.getTable().equals(ri.getTable()); } - @Override - public void close() throws IOException { + public void done() { // This is a table transition... after the last region. Check previous. // Should be last region. If not, its a hole on end of laster table. if (this.previous != null && !this.previous.isLast()) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSimpleMetaSplitMerge.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSimpleMetaSplitMerge.java index 7cbd245..58ec5a5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSimpleMetaSplitMerge.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestSimpleMetaSplitMerge.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.List; @@ -31,6 +32,7 @@ import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.master.assignment.RegionStateStore; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HStore; import org.apache.hadoop.hbase.testclassification.MediumTests; @@ -95,6 +97,20 @@ public class TestSimpleMetaSplitMerge { } } + private void compactMeta() throws IOException { + for (JVMClusterUtil.RegionServerThread t : UTIL.getMiniHBaseCluster() + .getRegionServerThreads()) { + for (HRegion r : t.getRegionServer().getOnlineRegionsLocalContext()) { + if (TableName.isMetaTableName(r.getRegionInfo().getTable())) { + r.compact(true); + for (HStore store : r.getStores()) { + store.closeAndArchiveCompactedFiles(); + } + } + } + } + } + @Test public void test() throws Exception { try (Table table = UTIL.getConnection().getTable(TD1.getTableName())) { @@ -104,6 +120,8 @@ public class TestSimpleMetaSplitMerge { table.put(new Put(Bytes.toBytes("row2")).addColumn(CF, CQ, Bytes.toBytes("row2"))); } Admin admin = UTIL.getAdmin(); + // turn off catalog janitor + admin.catalogJanitorSwitch(false); // split meta admin.split(TableName.META_TABLE_NAME, Bytes.toBytes("b")); assertMetaRegionCount(2); @@ -115,17 +133,7 @@ public class TestSimpleMetaSplitMerge { List<RegionInfo> regions = admin.getRegions(TableName.META_TABLE_NAME); assertEquals(2, regions.size()); // compact to make sure we can merge - for (JVMClusterUtil.RegionServerThread t : UTIL.getMiniHBaseCluster() - .getRegionServerThreads()) { - for (HRegion r : t.getRegionServer().getOnlineRegionsLocalContext()) { - if (TableName.isMetaTableName(r.getRegionInfo().getTable())) { - r.compact(true); - for (HStore store : r.getStores()) { - store.closeAndArchiveCompactedFiles(); - } - } - } - } + compactMeta(); // merge the 2 regions back to 1 admin.mergeRegionsAsync(regions.stream().map(RegionInfo::getRegionName).toArray(byte[][]::new), false).get(); @@ -137,5 +145,17 @@ public class TestSimpleMetaSplitMerge { // make sure that we could still get the locations from the new meta region assertValue(TD2.getTableName(), "row2"); assertValue(TD1.getTableName(), "row1"); + + // make sure that catalog janitor can clean up the merged regions + RegionStateStore regionStateStore = + UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionStateStore(); + RegionInfo mergedRegion = admin.getRegions(TableName.META_TABLE_NAME).get(0); + assertTrue(regionStateStore.hasMergeRegions(mergedRegion)); + // compact to make sure we could clean the merged regions + compactMeta(); + // one for merged region, one for split parent + assertEquals(2, admin.runCatalogJanitor()); + // the gc procedure is run in background so we need to wait here, can not check directly + UTIL.waitFor(30000, () -> !regionStateStore.hasMergeRegions(mergedRegion)); } }
