Skip to content

Commit

Permalink
HBASE-24370 Avoid aggressive MergeRegion and GCMultipleMergedRegionsP…
Browse files Browse the repository at this point in the history 
…rocedure
  • Loading branch information
Huaxiang Sun committed May 15, 2020
1 parent 94f36fd commit 1fbe693
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 14 deletions.
5 changes: 4 additions & 1 deletion hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,10 @@ public boolean cleanMergeQualifier(final RegionInfo region) throws IOException {
// It doesn't have merge qualifier, no need to clean
return true;
}
return cleanMergeRegion(region, parents);
// If a parent region is a merged child region and GC has not kicked in/finish its work yet,
// return false in this case to avoid kicking in a merge, trying later.
cleanMergeRegion(region, parents);
return false;
}

/**
Expand Down
37 changes: 28 additions & 9 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNotFoundException;
import org.apache.hadoop.hbase.exceptions.MergeRegionException;
import org.apache.hadoop.hbase.master.CatalogJanitor;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.regionserver.DisabledRegionSplitPolicy;
import org.apache.hadoop.hbase.regionserver.HRegion;
import org.apache.hadoop.hbase.regionserver.HStore;
Expand Down Expand Up @@ -530,25 +532,42 @@ public void testMergeRegions() throws Exception {
List<RegionInfo> tableRegions;
RegionInfo regionA;
RegionInfo regionB;
RegionInfo regionC;
RegionInfo mergedChildRegion = null;

// merge with full name
tableRegions = ADMIN.getRegions(tableName);
assertEquals(3, ADMIN.getRegions(tableName).size());
regionA = tableRegions.get(0);
regionB = tableRegions.get(1);
regionC = tableRegions.get(2);
// TODO convert this to version that is synchronous (See HBASE-16668)
ADMIN.mergeRegionsAsync(regionA.getRegionName(), regionB.getRegionName(), false).get(60,
TimeUnit.SECONDS);
ADMIN.mergeRegionsAsync(regionA.getRegionName(), regionB.getRegionName(),
false).get(60, TimeUnit.SECONDS);

assertEquals(2, ADMIN.getRegions(tableName).size());

// merge with encoded name
tableRegions = ADMIN.getRegions(tableName);
regionA = tableRegions.get(0);
regionB = tableRegions.get(1);

assertEquals(2, tableRegions.size());
for (RegionInfo ri : tableRegions) {
if (regionC.compareTo(ri) != 0) {
mergedChildRegion = ri;
break;
}
}

assertTrue(mergedChildRegion != null);
// Need to wait GC for merged child region is done.
HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
CatalogJanitor cj = services.getCatalogJanitor();
cj.cleanMergeQualifier(mergedChildRegion);
// Wait until all procedures settled down
while (!services.getMasterProcedureExecutor().getActiveProcIds().isEmpty()) {
Thread.sleep(200);
}

// TODO convert this to version that is synchronous (See HBASE-16668)
ADMIN
.mergeRegionsAsync(regionA.getEncodedNameAsBytes(), regionB.getEncodedNameAsBytes(), false)
ADMIN.mergeRegionsAsync(regionC.getEncodedNameAsBytes(),
mergedChildRegion.getEncodedNameAsBytes(), false)
.get(60, TimeUnit.SECONDS);

assertEquals(1, ADMIN.getRegions(tableName).size());
Expand Down
27 changes: 24 additions & 3 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.master.CatalogJanitor;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.testclassification.ClientTests;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
Expand Down Expand Up @@ -161,20 +163,39 @@ public void testMergeRegions() throws Exception {
.getTableHRegionLocations(metaTable, tableName).get();
RegionInfo regionA;
RegionInfo regionB;
RegionInfo regionC;
RegionInfo mergedChildRegion = null;

// merge with full name
assertEquals(3, regionLocations.size());
regionA = regionLocations.get(0).getRegion();
regionB = regionLocations.get(1).getRegion();
regionC = regionLocations.get(2).getRegion();
admin.mergeRegions(regionA.getRegionName(), regionB.getRegionName(), false).get();

regionLocations = AsyncMetaTableAccessor
.getTableHRegionLocations(metaTable, tableName).get();

assertEquals(2, regionLocations.size());
for (HRegionLocation rl : regionLocations) {
if (regionC.compareTo(rl.getRegion()) != 0) {
mergedChildRegion = rl.getRegion();
break;
}
}

assertTrue(mergedChildRegion != null);
// Need to wait GC for merged child region is done.
HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
CatalogJanitor cj = services.getCatalogJanitor();
cj.cleanMergeQualifier(mergedChildRegion);
// Wait until all procedures settled down
while (!services.getMasterProcedureExecutor().getActiveProcIds().isEmpty()) {
Thread.sleep(200);
}
// merge with encoded name
regionA = regionLocations.get(0).getRegion();
regionB = regionLocations.get(1).getRegion();
admin.mergeRegions(regionA.getRegionName(), regionB.getRegionName(), false).get();
admin.mergeRegions(regionC.getRegionName(), mergedChildRegion.getRegionName(),
false).get();

regionLocations = AsyncMetaTableAccessor
.getTableHRegionLocations(metaTable, tableName).get();
Expand Down
77 changes: 76 additions & 1 deletion hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@
import java.util.List;
import java.util.Map;
import java.util.function.BooleanSupplier;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellBuilderFactory;
import org.apache.hadoop.hbase.CellBuilderType;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.Result;
Expand All @@ -43,6 +47,7 @@
import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.Pair;
import org.apache.hadoop.hbase.util.Threads;
import org.junit.AfterClass;
Expand Down Expand Up @@ -141,7 +146,7 @@ public void testOneRegionTable() throws IOException {
assertEquals(0, ris.size());
}

private static void makeOverlap(MasterServices services, RegionInfo a, RegionInfo b)
private static RegionInfo makeOverlap(MasterServices services, RegionInfo a, RegionInfo b)
throws IOException {
RegionInfo overlapRegion = RegionInfoBuilder.newBuilder(a.getTable()).
setStartKey(a.getStartKey()).
Expand All @@ -152,6 +157,7 @@ private static void makeOverlap(MasterServices services, RegionInfo a, RegionInf
System.currentTimeMillis())));
// TODO: Add checks at assign time to PREVENT being able to assign over existing assign.
services.getAssignmentManager().assign(overlapRegion);
return overlapRegion;
}

private void testOverlapCommon(final TableName tn) throws Exception {
Expand Down Expand Up @@ -308,6 +314,75 @@ public void testOverlapWithSmallMergeCount() throws Exception {
}
}

/**
* This test covers the case that one of merged parent regions is a merged child region that
* has not been GCed but there is no reference files anymore. In this case, it will kick off
* a GC procedure, but no merge will happen.
*/
@Test
public void testMergeWithMergedChildRegion() throws Exception {
TableName tn = TableName.valueOf(this.name.getMethodName());
Table t = TEST_UTIL.createMultiRegionTable(tn, HConstants.CATALOG_FAMILY);
List<RegionInfo> ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn);
assertTrue(ris.size() > 5);
HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
CatalogJanitor cj = services.getCatalogJanitor();
cj.scan();
CatalogJanitor.Report report = cj.getLastReport();
assertTrue(report.isEmpty());
RegionInfo overlapRegion = makeOverlap(services, ris.get(1), ris.get(2));
Threads.sleep(10000);

cj.scan();
report = cj.getLastReport();
assertEquals(2, report.getOverlaps().size());

// Mark it as a merged child region.
RegionInfo fakedParentRegion = RegionInfoBuilder.newBuilder(tn).
setStartKey(overlapRegion.getStartKey()).
build();

Table meta = MetaTableAccessor.getMetaHTable(TEST_UTIL.getConnection());
long time = HConstants.LATEST_TIMESTAMP;
Put putOfMerged = MetaTableAccessor.makePutFromRegionInfo(overlapRegion, time);
String qualifier = String.format(HConstants.MERGE_QUALIFIER_PREFIX_STR + "%04d", 0);
putOfMerged.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY).setRow(
putOfMerged.getRow()).
setFamily(HConstants.CATALOG_FAMILY).
setQualifier(Bytes.toBytes(qualifier)).
setTimestamp(putOfMerged.getTimestamp()).
setType(Cell.Type.Put).
setValue(RegionInfo.toByteArray(fakedParentRegion)).
build());

meta.put(putOfMerged);

MetaFixer fixer = new MetaFixer(services);
fixer.fixOverlaps(report);

// Wait until all procedures settled down
await(200, () -> {
return services.getMasterProcedureExecutor().getActiveProcIds().isEmpty();
});

// No merge is done, overlap is still there.
cj.scan();
report = cj.getLastReport();
assertEquals(2, report.getOverlaps().size());

fixer.fixOverlaps(report);

// Wait until all procedures settled down
await(200, () -> {
return services.getMasterProcedureExecutor().getActiveProcIds().isEmpty();
});

// Merge is done and no more overlaps
cj.scan();
report = cj.getLastReport();
assertEquals(0, report.getOverlaps().size());
}

/**
* Make it so a big overlap spans many Regions, some of which are non-contiguous. Make it so
* we can fix this condition. HBASE-24247
Expand Down

0 comments on commit 1fbe693

Please sign in to comment.