Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-24995: MetaFixer fails to fix overlaps when multiple tables hav… #2361

Merged
merged 1 commit into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -231,16 +235,18 @@ private static List<RegionInfo> createMetaEntries(final MasterServices masterSer
/**
* Fix overlaps noted in CJ consistency report.
*/
void fixOverlaps(CatalogJanitor.Report report) throws IOException {
List<Long> fixOverlaps(CatalogJanitor.Report report) throws IOException {
List<Long> pidList = new ArrayList<>();
arshadmohammad marked this conversation as resolved.
Show resolved Hide resolved
for (Set<RegionInfo> 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;
}

/**
Expand All @@ -258,6 +264,21 @@ static List<SortedSet<RegionInfo>> calculateMerges(int maxMergeCount,
return Collections.emptyList();
}
List<SortedSet<RegionInfo>> merges = new ArrayList<>();
// First group overlaps by table then calculate merge table by table.
ListMultimap<TableName, Pair<RegionInfo, RegionInfo>> overlapGroups =
ArrayListMultimap.create();
for (Pair<RegionInfo, RegionInfo> pair : overlaps) {
overlapGroups.put(pair.getFirst().getTable(), pair);
}
for (Map.Entry<TableName, Collection<Pair<RegionInfo, RegionInfo>>> entry : overlapGroups
.asMap().entrySet()) {
calculateTableMerges(maxMergeCount, merges, entry.getValue());
}
return merges;
}

private static void calculateTableMerges(int maxMergeCount, List<SortedSet<RegionInfo>> merges,
Collection<Pair<RegionInfo, RegionInfo>> overlaps) {
SortedSet<RegionInfo> currentMergeSet = new TreeSet<>();
HashSet<RegionInfo> regionsInMergeSet = new HashSet<>();
RegionInfo regionInfoWithlargestEndKey = null;
Expand Down Expand Up @@ -301,7 +322,6 @@ static List<SortedSet<RegionInfo>> calculateMerges(int maxMergeCount,
regionInfoWithlargestEndKey);
}
merges.add(currentMergeSet);
return merges;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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<RegionInfo> 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<Long> 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());
Expand Down