You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2020/10/22 14:24:09 UTC
[hbase] branch branch-2.2 updated: HBASE-25207 Revisit the
implementation and usage of RegionStates.include (#2571)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-2.2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.2 by this push:
new 4e525e5 HBASE-25207 Revisit the implementation and usage of RegionStates.include (#2571)
4e525e5 is described below
commit 4e525e567e8b27a89796e491353bb72629829b9e
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Thu Oct 22 21:50:43 2020 +0800
HBASE-25207 Revisit the implementation and usage of RegionStates.include (#2571)
Remove the RegionStates.include method as its name is ambiguous.
Add more comments to describe the logic on why we filter region like
this.
Signed-off-by: Toshihiro Suzuki <br...@gmail.com>
---
.../hbase/master/assignment/AssignmentManager.java | 17 +++++++++++--
.../hbase/master/assignment/RegionStates.java | 28 ++++++++++++++--------
.../master/procedure/EnableTableProcedure.java | 8 ++-----
.../hbase/master/assignment/TestRegionStates.java | 23 +++++++-----------
4 files changed, 44 insertions(+), 32 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
index 0c6e03e..ac8fd21 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
@@ -842,8 +842,13 @@ public class AssignmentManager {
return regionStates.getTableRegionStateNodes(tableName).stream().map(regionNode -> {
regionNode.lock();
try {
- if (!regionStates.include(regionNode, false) ||
- regionStates.isRegionOffline(regionNode.getRegionInfo())) {
+ if (regionNode.isInState(State.OFFLINE, State.CLOSED, State.SPLIT)) {
+ return null;
+ }
+ // in general, a split parent should be in CLOSED or SPLIT state, but anyway, let's check it
+ // here for safety
+ if (regionNode.getRegionInfo().isSplit()) {
+ LOG.warn("{} is a split parent but not in CLOSED or SPLIT state", regionNode);
return null;
}
// As in DisableTableProcedure, we will hold the xlock for table, so we can make sure that
@@ -1800,6 +1805,14 @@ public class AssignmentManager {
final RegionStateNode nodeB = regionStates.getOrCreateRegionStateNode(daughterB);
nodeB.setState(State.SPLITTING_NEW);
+ // TODO: here we just update the parent region info in meta, to set split and offline to true,
+ // without changing the one in the region node. This is a bit confusing but the region info
+ // field in RegionStateNode is not expected to be changed in the current design. Need to find a
+ // possible way to address this problem, or at least adding more comments about the trick to
+ // deal with this problem, that when you want to filter out split parent, you need to check both
+ // the RegionState on whether it is split, and also the region info. If one of them matches then
+ // it is a split parent. And usually only one of them can match, as after restart, the region
+ // state will be changed from SPLIT to CLOSED.
regionStateStore.splitRegion(parent, daughterA, daughterB, serverName);
if (shouldAssignFavoredNodes(parent)) {
List<ServerName> onlineServers = this.master.getServerManager().getOnlineServersList();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
index 019f117..1bc1987 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
@@ -150,7 +150,7 @@ public class RegionStates {
regionInfos.forEach(this::deleteRegion);
}
- ArrayList<RegionStateNode> getTableRegionStateNodes(final TableName tableName) {
+ List<RegionStateNode> getTableRegionStateNodes(final TableName tableName) {
final ArrayList<RegionStateNode> regions = new ArrayList<RegionStateNode>();
for (RegionStateNode node: regionsMap.tailMap(tableName.getName()).values()) {
if (!node.getTable().equals(tableName)) break;
@@ -220,8 +220,10 @@ public class RegionStates {
/**
* @return Return online regions of table; does not include OFFLINE or SPLITTING regions.
*/
- public List<RegionInfo> getRegionsOfTable(final TableName table) {
- return getRegionsOfTable(table, false);
+ public List<RegionInfo> getRegionsOfTable(TableName table) {
+ return getRegionsOfTable(table,
+ regionNode -> !regionNode.isInState(State.OFFLINE, State.SPLIT) &&
+ !regionNode.getRegionInfo().isSplitParent());
}
private HRegionLocation createRegionForReopen(RegionStateNode node) {
@@ -325,16 +327,22 @@ public class RegionStates {
}
/**
- * @return Return online regions of table; does not include OFFLINE or SPLITTING regions.
+ * Get the regions for enabling a table.
+ * <p/>
+ * Here we want the EnableTableProcedure to be more robust and can be used to fix some nasty
+ * states, so the checks in this method will be a bit strange. In general, a region can only be
+ * offline when it is split, for merging we will just delete the parent regions, but with HBCK we
+ * may force update the state of a region to fix some nasty bugs, so in this method we will try to
+ * bring the offline regions back if it is not split. That's why we only check for split state
+ * here.
*/
- public List<RegionInfo> getRegionsOfTable(TableName table, boolean offline) {
- return getRegionsOfTable(table, state -> include(state, offline));
+ public List<RegionInfo> getRegionsOfTableForEnabling(TableName table) {
+ return getRegionsOfTable(table,
+ regionNode -> !regionNode.isInState(State.SPLIT) && !regionNode.getRegionInfo().isSplit());
}
/**
- * @return Return the regions of the table; does not include OFFLINE unless you set
- * <code>offline</code> to true. Does not include regions that are in the
- * {@link State#SPLIT} state.
+ * @return Return the regions of the table and filter them.
*/
private List<RegionInfo> getRegionsOfTable(TableName table, Predicate<RegionStateNode> filter) {
return getTableRegionStateNodes(table).stream().filter(filter).map(n -> n.getRegionInfo())
@@ -347,7 +355,7 @@ public class RegionStates {
* @return True if we should include the <code>node</code> (do not include
* if split or offline unless <code>offline</code> is set to true.
*/
- boolean include(final RegionStateNode node, final boolean offline) {
+ private boolean include(final RegionStateNode node, final boolean offline) {
if (LOG.isTraceEnabled()) {
LOG.trace("WORKING ON " + node + " " + node.getRegionInfo());
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
index 06d6a2c..b3dd917 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
@@ -104,14 +104,10 @@ public class EnableTableProcedure
// count
TableDescriptor hTableDescriptor =
env.getMasterServices().getTableDescriptors().get(tableName);
-
- // Get the replica count
int regionReplicaCount = hTableDescriptor.getRegionReplication();
-
- // Get the regions for the table from memory; get both online and offline regions
- // ('true').
+ // Get regions for the table from memory
List<RegionInfo> regionsOfTable =
- env.getAssignmentManager().getRegionStates().getRegionsOfTable(tableName, true);
+ env.getAssignmentManager().getRegionStates().getRegionsOfTableForEnabling(tableName);
int currentMaxReplica = 0;
// Check if the regions in memory have replica regions as marked in META table
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStates.java
index 6f12b95..b127899 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStates.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStates.java
@@ -58,7 +58,7 @@ public class TestRegionStates {
protected static final HBaseTestingUtility UTIL = new HBaseTestingUtility();
private static ThreadPoolExecutor threadPool;
- private static ExecutorCompletionService executorService;
+ private static ExecutorCompletionService<Object> executorService;
@BeforeClass
public static void setUp() throws Exception {
@@ -70,7 +70,7 @@ public class TestRegionStates {
LOG.warn("Failed thread " + t.getName(), e);
}
}));
- executorService = new ExecutorCompletionService(threadPool);
+ executorService = new ExecutorCompletionService<>(threadPool);
}
@AfterClass
@@ -133,13 +133,13 @@ public class TestRegionStates {
checkTableRegions(stateMap, TABLE_NAME_C, NSMALL_RUNS);
}
- private void checkTableRegions(final RegionStates stateMap,
- final TableName tableName, final int nregions) {
- List<RegionInfo> hris = stateMap.getRegionsOfTable(tableName, true);
- assertEquals(nregions, hris.size());
- for (int i = 1; i < hris.size(); ++i) {
- long a = Bytes.toLong(hris.get(i - 1).getStartKey());
- long b = Bytes.toLong(hris.get(i + 0).getStartKey());
+ private void checkTableRegions(final RegionStates stateMap, final TableName tableName,
+ final int nregions) {
+ List<RegionStateNode> rns = stateMap.getTableRegionStateNodes(tableName);
+ assertEquals(nregions, rns.size());
+ for (int i = 1; i < rns.size(); ++i) {
+ long a = Bytes.toLong(rns.get(i - 1).getRegionInfo().getStartKey());
+ long b = Bytes.toLong(rns.get(i + 0).getRegionInfo().getStartKey());
assertEquals(b, a + 1);
}
}
@@ -159,11 +159,6 @@ public class TestRegionStates {
});
}
- private Object createRegionNode(final RegionStates stateMap,
- final TableName tableName, final long regionId) {
- return stateMap.getOrCreateRegionStateNode(createRegionInfo(tableName, regionId));
- }
-
private RegionInfo createRegionInfo(final TableName tableName, final long regionId) {
return RegionInfoBuilder.newBuilder(tableName)
.setStartKey(Bytes.toBytes(regionId))