You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by hu...@apache.org on 2020/05/06 06:05:01 UTC
[hbase] branch master updated: HBASE-24316
GCMulitpleMergedRegionsProcedure is not idempotent (#1660)
This is an automated email from the ASF dual-hosted git repository.
huaxiangsun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 045c909 HBASE-24316 GCMulitpleMergedRegionsProcedure is not idempotent (#1660)
045c909 is described below
commit 045c909bdfc906b2687a6b7ba86c290aa839d7c6
Author: huaxiangsun <hu...@apache.org>
AuthorDate: Tue May 5 23:04:48 2020 -0700
HBASE-24316 GCMulitpleMergedRegionsProcedure is not idempotent (#1660)
It addresses couple issues:
1. Make sure deleteMergeQualifiers() does not delete the row if there is no columns with "merge" keyword.
2. GCMulitpleMergedRegionsProcedure now acquire an exclusive lock on the child region.
Signed-off-by: stack <st...@apache.org>
---
.../org/apache/hadoop/hbase/MetaTableAccessor.java | 10 ++++++
.../GCMultipleMergedRegionsProcedure.java | 20 ++++++++++++
.../apache/hadoop/hbase/master/TestMetaFixer.java | 37 ++++++++++++++++++++--
3 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
index 113f22c..dec1bca 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
@@ -1846,6 +1846,16 @@ public class MetaTableAccessor {
qualifiers.add(qualifier);
delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
}
+
+ // There will be race condition that a GCMultipleMergedRegionsProcedure is scheduled while
+ // the previous GCMultipleMergedRegionsProcedure is still going on, in this case, the second
+ // GCMultipleMergedRegionsProcedure could delete the merged region by accident!
+ if (qualifiers.isEmpty()) {
+ LOG.info("No merged qualifiers for region " + mergeRegion.getRegionNameAsString() +
+ " in meta table, they are cleaned up already, Skip.");
+ return;
+ }
+
deleteFromMetaTable(connection, delete);
LOG.info("Deleted merge references in " + mergeRegion.getRegionNameAsString() +
", deleted qualifiers " + qualifiers.stream().map(Bytes::toStringBinary).
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
index 285891e..8042d61 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java
@@ -63,6 +63,26 @@ public class GCMultipleMergedRegionsProcedure extends
super();
}
+ @Override protected boolean holdLock(MasterProcedureEnv env) {
+ return true;
+ }
+
+ @Override
+ protected LockState acquireLock(final MasterProcedureEnv env) {
+ // It now takes an exclusive lock on the merged child region to make sure
+ // that no two parallel running of two GCMultipleMergedRegionsProcedures on the
+ // region.
+ if (env.getProcedureScheduler().waitRegion(this, mergedChild)) {
+ return LockState.LOCK_EVENT_WAIT;
+ }
+ return LockState.LOCK_ACQUIRED;
+ }
+
+ @Override
+ protected void releaseLock(final MasterProcedureEnv env) {
+ env.getProcedureScheduler().wakeRegion(this, mergedChild);
+ }
+
@Override
public TableOperationType getTableOperationType() {
return TableOperationType.MERGED_REGIONS_GC;
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 ee67110..8d080e8 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
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.function.BooleanSupplier;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
@@ -31,8 +32,12 @@ import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
+import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure;
+import org.apache.hadoop.hbase.master.assignment.GCMultipleMergedRegionsProcedure;
+import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
+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.Threads;
@@ -168,18 +173,44 @@ public class TestMetaFixer {
assertEquals(1, MetaFixer.calculateMerges(10, report.getOverlaps()).size());
MetaFixer fixer = new MetaFixer(services);
fixer.fixOverlaps(report);
+
+ CatalogJanitor cj = services.getCatalogJanitor();
await(10, () -> {
try {
- services.getCatalogJanitor().scan();
- final CatalogJanitor.Report postReport = services.getCatalogJanitor().getLastReport();
- return postReport.isEmpty();
+ if (cj.scan() > 0) {
+ // It submits GC once, then it will immediately kick off another GC to test if
+ // GCMultipleMergedRegionsProcedure is idempotent. If it is not, it will create
+ // a hole.
+ Map<RegionInfo, Result> mergedRegions = cj.getLastReport().mergedRegions;
+ for (Map.Entry<RegionInfo, Result> e : mergedRegions.entrySet()) {
+ List<RegionInfo> parents = MetaTableAccessor.getMergeRegions(e.getValue().rawCells());
+ if (parents != null) {
+ ProcedureExecutor<MasterProcedureEnv> pe = services.getMasterProcedureExecutor();
+ pe.submitProcedure(new GCMultipleMergedRegionsProcedure(pe.getEnvironment(),
+ e.getKey(), parents));
+ }
+ }
+ return true;
+ }
+ return false;
} catch (Exception e) {
throw new RuntimeException(e);
}
});
+ // Wait until all GCs settled down
+ await(10, () -> {
+ return services.getMasterProcedureExecutor().getActiveProcIds().isEmpty();
+ });
+
+ // No orphan regions on FS
hbckChore.chore();
assertEquals(0, hbckChore.getOrphanRegionsOnFS().size());
+
+ // No holes reported.
+ cj.scan();
+ final CatalogJanitor.Report postReport = cj.getLastReport();
+ assertTrue(postReport.isEmpty());
}
/**