From 2250b51fe7b5ac84f9019e1df082da47918d7514 Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Wed, 9 Sep 2020 00:30:44 +0530 Subject: [PATCH] HBASE-24995: MetaFixer fails to fix overlaps when multiple tables have overlaps (#2361) Signed-off-by: stack --- .../apache/hadoop/hbase/master/MetaFixer.java | 28 +++++++++++-- .../hadoop/hbase/master/TestMetaFixer.java | 39 ++++++++++++++++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java index cc8a3db4e937..ed1fd27a1c4e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java @@ -19,9 +19,11 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.SortedSet; @@ -39,6 +41,8 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil; +import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap; +import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -231,16 +235,18 @@ private static List createMetaEntries(final MasterServices masterSer /** * Fix overlaps noted in CJ consistency report. */ - void fixOverlaps(CatalogJanitor.Report report) throws IOException { + List fixOverlaps(CatalogJanitor.Report report) throws IOException { + List pidList = new ArrayList<>(); for (Set regions: calculateMerges(maxMergeCount, report.getOverlaps())) { RegionInfo [] regionsArray = regions.toArray(new RegionInfo [] {}); try { - this.masterServices.mergeRegions(regionsArray, - true, HConstants.NO_NONCE, HConstants.NO_NONCE); + pidList.add(this.masterServices + .mergeRegions(regionsArray, true, HConstants.NO_NONCE, HConstants.NO_NONCE)); } catch (MergeRegionException mre) { LOG.warn("Failed overlap fix of {}", regionsArray, mre); } } + return pidList; } /** @@ -258,6 +264,21 @@ static List> calculateMerges(int maxMergeCount, return Collections.emptyList(); } List> merges = new ArrayList<>(); + // First group overlaps by table then calculate merge table by table. + ListMultimap> overlapGroups = + ArrayListMultimap.create(); + for (Pair pair : overlaps) { + overlapGroups.put(pair.getFirst().getTable(), pair); + } + for (Map.Entry>> entry : overlapGroups + .asMap().entrySet()) { + calculateTableMerges(maxMergeCount, merges, entry.getValue()); + } + return merges; + } + + private static void calculateTableMerges(int maxMergeCount, List> merges, + Collection> overlaps) { SortedSet currentMergeSet = new TreeSet<>(); HashSet regionsInMergeSet = new HashSet<>(); RegionInfo regionInfoWithlargestEndKey = null; @@ -301,7 +322,6 @@ static List> calculateMerges(int maxMergeCount, regionInfoWithlargestEndKey); } merges.add(currentMergeSet); - return merges; } /** diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java index 669259942ff5..3b751664332c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java @@ -46,6 +46,7 @@ import org.apache.hadoop.hbase.master.assignment.RegionStates; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; +import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.util.Bytes; @@ -171,7 +172,8 @@ private static RegionInfo makeOverlap(MasterServices services, RegionInfo a, Reg Collections.singletonList(MetaTableAccessor.makePutFromRegionInfo(overlapRegion, System.currentTimeMillis()))); // TODO: Add checks at assign time to PREVENT being able to assign over existing assign. - services.getAssignmentManager().assign(overlapRegion); + long assign = services.getAssignmentManager().assign(overlapRegion); + ProcedureTestingUtility.waitProcedures(services.getMasterProcedureExecutor(), assign); return overlapRegion; } @@ -244,6 +246,41 @@ HBaseTestingUtility. await(10, () -> { assertTrue(postReport.isEmpty()); } + @Test + public void testMultipleTableOverlaps() throws Exception { + TableName t1 = TableName.valueOf("t1"); + TableName t2 = TableName.valueOf("t2"); + TEST_UTIL.createMultiRegionTable(t1, new byte[][] { HConstants.CATALOG_FAMILY }); + TEST_UTIL.createMultiRegionTable(t2, new byte[][] { HConstants.CATALOG_FAMILY }); + TEST_UTIL.waitTableAvailable(t2); + + HMaster services = TEST_UTIL.getHBaseCluster().getMaster(); + services.getCatalogJanitor().scan(); + CatalogJanitor.Report report = services.getCatalogJanitor().getLastReport(); + assertTrue(report.isEmpty()); + + // Make a simple overlap for t1 + List ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), t1); + makeOverlap(services, ris.get(1), ris.get(2)); + // Make a simple overlap for t2 + ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), t2); + makeOverlap(services, ris.get(1), ris.get(2)); + + services.getCatalogJanitor().scan(); + report = services.getCatalogJanitor().getLastReport(); + assertEquals("Region overlaps count does not match.", 4, report.getOverlaps().size()); + + MetaFixer fixer = new MetaFixer(services); + List longs = fixer.fixOverlaps(report); + long[] procIds = longs.stream().mapToLong(l -> l).toArray(); + ProcedureTestingUtility.waitProcedures(services.getMasterProcedureExecutor(), procIds); + + // After fix, verify no overlaps are left. + services.getCatalogJanitor().scan(); + report = services.getCatalogJanitor().getLastReport(); + assertTrue("After fix there should not have been any overlaps.", report.isEmpty()); + } + @Test public void testOverlapWithSmallMergeCount() throws Exception { TableName tn = TableName.valueOf(this.name.getMethodName());