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 <zhangduo@apache.org>
Signed-off-by: Michael Stack <stack@apache.org>
Signed-off-by: Viraj Jasani <vjasani@apache.org>
This commit is contained in:
Andrew Purtell 2021-05-06 09:13:33 -07:00 committed by GitHub
parent ba4cb91211
commit 6309c090b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 10 deletions

View File

@ -280,12 +280,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;
@ -356,10 +350,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()) {

View File

@ -33,6 +33,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;
@ -44,6 +45,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;
@ -120,6 +125,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",