This is an automated email from the ASF dual-hosted git repository.
apurtell pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.4 by this push:
new 4de3841 HBASE-25854 Remove redundant AM in-memory state changes in
CatalogJanitor (#3234)
4de3841 is described below
commit 4de3841074615331bf077a75bf11bad12ee80528
Author: Andrew Purtell <[email protected]>
AuthorDate: Thu May 6 09:13:33 2021 -0700
HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
(#3234)
In CatalogJanitor we schedule GCRegionProcedure to clean up both
filesystem and in-memory state after a split, and
GCMultipleMergedRegionsProcedure to do the same for merges. Both of these
procedures clean up in-memory state, but CatalogJanitor also does this
redundantly just after scheduling the procedures. The cleanup should be
done in only one place. Presumably we are using the procedures to do it in
a principled way. Remove the redundancy in CatalogJanitor and fix any
follow on issues, like test failures.
Signed-off-by: Duo Zhang <[email protected]>
Signed-off-by: Michael Stack <[email protected]>
Signed-off-by: Viraj Jasani <[email protected]>
---
.../hadoop/hbase/master/janitor/CatalogJanitor.java | 10 ----------
.../janitor/TestCatalogJanitorInMemoryStates.java | 21 +++++++++++++++++++++
2 files changed, 21 insertions(+), 10 deletions(-)
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 1674bb0..ef6145b 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
@@ -279,12 +279,6 @@ public class CatalogJanitor extends ScheduledChore {
LOG.debug("Submitted procedure {} for merged region {}",
mergeRegionProcedure,
mergedRegion);
}
- // TODO: The above scheduled GCMultipleMergedRegionsProcedure does the
below.
- // Do we need this?
- for (RegionInfo ri : parents) {
-
this.services.getAssignmentManager().getRegionStates().deleteRegion(ri);
- this.services.getServerManager().removeRegion(ri);
- }
return true;
}
return false;
@@ -355,10 +349,6 @@ public class CatalogJanitor extends ScheduledChore {
if (LOG.isDebugEnabled()) {
LOG.debug("Submitted procedure {} for split parent {}",
gcRegionProcedure, parent);
}
- // Remove from in-memory states
- // TODO: The above scheduled GCRegionProcedure does the below. Do we
need this?
- services.getAssignmentManager().getRegionStates().deleteRegion(parent);
- services.getServerManager().removeRegion(parent);
return true;
} else {
if (LOG.isDebugEnabled()) {
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorInMemoryStates.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorInMemoryStates.java
index d5348fd..d608480 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorInMemoryStates.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitorInMemoryStates.java
@@ -32,6 +32,7 @@ import org.apache.hadoop.hbase.MetaMockingUtil;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNameTestRule;
+import org.apache.hadoop.hbase.Waiter;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
@@ -43,6 +44,10 @@ import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.ServerManager;
import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
+import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure;
+import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
+import org.apache.hadoop.hbase.procedure2.Procedure;
+import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.util.Bytes;
@@ -119,6 +124,22 @@ public class TestCatalogJanitorInMemoryStates {
Result r = MetaMockingUtil.getMetaTableRowResult(parent.getRegion(), null,
daughters.get(0).getRegion(), daughters.get(1).getRegion());
CatalogJanitor.cleanParent(master, parent.getRegion(), r);
+
+ // wait for procedures to complete
+ Waiter.waitFor(TEST_UTIL.getConfiguration(), 10 * 1000, new
Waiter.Predicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> pe =
master.getMasterProcedureExecutor();
+ for (Procedure<MasterProcedureEnv> proc: pe.getProcedures()) {
+ if (proc.getClass().isAssignableFrom(GCRegionProcedure.class) &&
+ proc.isFinished()) {
+ return true;
+ }
+ }
+ return false;
+ }
+ });
+
assertFalse("Parent region should have been removed from RegionStates",
am.getRegionStates().isRegionInRegionStates(parent.getRegion()));
assertFalse("Parent region should have been removed from ServerManager",
