apurtell commented on code in PR #7375:
URL: https://github.com/apache/hbase/pull/7375#discussion_r2430736909
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java:
##########
@@ -2046,15 +2057,56 @@ public Pair<Integer, Integer> getReopenStatus(TableName
tableName) {
return new Pair<Integer, Integer>(ritCount, states.size());
}
+ // This comparator sorts the RegionStates by time stamp then Region name.
+ // Comparing by timestamp alone can lead us to discard different
RegionStates that happen
+ // to share a timestamp.
+ private static class RegionStateStampComparator implements
Comparator<RegionState> {
+ @Override
+ public int compare(final RegionState l, final RegionState r) {
+ int stampCmp = Long.compare(l.getStamp(), r.getStamp());
+ return stampCmp != 0 ? stampCmp :
RegionInfo.COMPARATOR.compare(l.getRegion(), r.getRegion());
+ }
+ }
+
+ public final static RegionStateStampComparator REGION_STATE_STAMP_COMPARATOR
=
+ new RegionStateStampComparator();
+
//
============================================================================================
// TODO: Region State In Transition
//
============================================================================================
public boolean hasRegionsInTransition() {
- return regionStates.hasRegionsInTransition();
+ return regionInTransitionTracker.hasRegionsInTransition();
}
public List<RegionStateNode> getRegionsInTransition() {
- return regionStates.getRegionsInTransition();
+ return regionInTransitionTracker.getRegionsInTransition();
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransitionTracker.isRegionInTransition(regionInfo);
+ }
+
+ public int getOngoingTRSPCount() {
Review Comment:
Call it `getInTransitCount()`?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java:
##########
@@ -2046,15 +2057,56 @@ public Pair<Integer, Integer> getReopenStatus(TableName
tableName) {
return new Pair<Integer, Integer>(ritCount, states.size());
}
+ // This comparator sorts the RegionStates by time stamp then Region name.
+ // Comparing by timestamp alone can lead us to discard different
RegionStates that happen
+ // to share a timestamp.
+ private static class RegionStateStampComparator implements
Comparator<RegionState> {
+ @Override
+ public int compare(final RegionState l, final RegionState r) {
+ int stampCmp = Long.compare(l.getStamp(), r.getStamp());
+ return stampCmp != 0 ? stampCmp :
RegionInfo.COMPARATOR.compare(l.getRegion(), r.getRegion());
+ }
+ }
+
+ public final static RegionStateStampComparator REGION_STATE_STAMP_COMPARATOR
=
+ new RegionStateStampComparator();
+
//
============================================================================================
// TODO: Region State In Transition
//
============================================================================================
public boolean hasRegionsInTransition() {
- return regionStates.hasRegionsInTransition();
+ return regionInTransitionTracker.hasRegionsInTransition();
}
public List<RegionStateNode> getRegionsInTransition() {
- return regionStates.getRegionsInTransition();
+ return regionInTransitionTracker.getRegionsInTransition();
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransitionTracker.isRegionInTransition(regionInfo);
+ }
+
+ public int getOngoingTRSPCount() {
+ return regionStates.getOngoingTRSPCount();
+ }
+
+ /**
+ * Get the number of regions in transition.
+ */
+ public int getRegionsInTransitionCount() {
+ return regionInTransitionTracker.getRegionsInTransition().size();
+ }
+
+ public List<RegionState> getRegionsStateInTransition() {
+ return
getRegionsInTransition().stream().map(RegionStateNode::toRegionState).toList();
+ }
+
+ public SortedSet<RegionState> getRegionsInTransitionOrderedByTimestamp() {
Review Comment:
Is there a reason not to always return the sorted set. Do we need to have
two methods, one for a sorted set and one for an unsorted list? (We don't,
right?) `getRegionsStateInTransition` uses a stream to build the list so it's
not much cheaper than building the treeset probably.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.assignment;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.TableStateManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+public class RegionInTransitionTracker {
+ private static final Logger LOG =
LoggerFactory.getLogger(RegionInTransitionTracker.class);
+
+ private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
+ List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
+
+ private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
List.of(RegionState.State.OPEN);
+
+ private final ConcurrentSkipListMap<RegionInfo, RegionStateNode>
regionInTransition =
+ new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
+ private final TableStateManager tableStateManager;
+
+ public RegionInTransitionTracker(TableStateManager tableStateManager) {
+ this.tableStateManager = tableStateManager;
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransition.containsKey(regionInfo);
+ }
+
+ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
+ RegionState.State currentState = regionStateNode.getState();
+ // only consider default replica for availability
Review Comment:
This is ok for now.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java:
##########
@@ -1794,6 +1803,7 @@ public void joinCluster() throws IOException {
*/
// Public so can be run by the Master as part of the startup. Needs
hbase:meta to be online.
// Needs to be done after the table state manager has been started.
+ // TODO umesh we can update the rit map here
Review Comment:
Remove this comment. Implement if necessary.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateNode.java:
##########
@@ -161,7 +161,7 @@ public boolean isStuck() {
return isInState(State.FAILED_OPEN) && getProcedure() != null;
}
- public boolean isInTransition() {
+ public boolean isOngoingTRSP() {
Review Comment:
How about `isInTransit`?
And javadoc that "transit" means something different than "in transition".
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java:
##########
@@ -2046,15 +2057,56 @@ public Pair<Integer, Integer> getReopenStatus(TableName
tableName) {
return new Pair<Integer, Integer>(ritCount, states.size());
}
+ // This comparator sorts the RegionStates by time stamp then Region name.
+ // Comparing by timestamp alone can lead us to discard different
RegionStates that happen
+ // to share a timestamp.
+ private static class RegionStateStampComparator implements
Comparator<RegionState> {
+ @Override
+ public int compare(final RegionState l, final RegionState r) {
+ int stampCmp = Long.compare(l.getStamp(), r.getStamp());
+ return stampCmp != 0 ? stampCmp :
RegionInfo.COMPARATOR.compare(l.getRegion(), r.getRegion());
+ }
+ }
+
+ public final static RegionStateStampComparator REGION_STATE_STAMP_COMPARATOR
=
+ new RegionStateStampComparator();
+
//
============================================================================================
// TODO: Region State In Transition
//
============================================================================================
public boolean hasRegionsInTransition() {
- return regionStates.hasRegionsInTransition();
+ return regionInTransitionTracker.hasRegionsInTransition();
}
public List<RegionStateNode> getRegionsInTransition() {
- return regionStates.getRegionsInTransition();
+ return regionInTransitionTracker.getRegionsInTransition();
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransitionTracker.isRegionInTransition(regionInfo);
+ }
+
+ public int getOngoingTRSPCount() {
Review Comment:
"Transit" means something different than "in transition". Javadoc the
explaination.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java:
##########
@@ -2303,12 +2363,14 @@ public void markRegionAsSplit(final RegionInfo parent,
final ServerName serverNa
public void markRegionAsMerged(final RegionInfo child, final ServerName
serverName,
RegionInfo[] mergeParents) throws IOException {
final RegionStateNode node =
regionStates.getOrCreateRegionStateNode(child);
- node.setState(State.MERGED);
for (RegionInfo ri : mergeParents) {
regionStates.deleteRegion(ri);
+ regionInTransitionTracker.handleRegionDelete(ri);
}
+ // TODO need to handle delete and new region
Review Comment:
Is this comment still relevant? Looks like the delete and new region
creations are handled.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.assignment;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.TableStateManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+public class RegionInTransitionTracker {
+ private static final Logger LOG =
LoggerFactory.getLogger(RegionInTransitionTracker.class);
+
+ private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
+ List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
+
+ private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
List.of(RegionState.State.OPEN);
+
+ private final ConcurrentSkipListMap<RegionInfo, RegionStateNode>
regionInTransition =
+ new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
+ private final TableStateManager tableStateManager;
+
+ public RegionInTransitionTracker(TableStateManager tableStateManager) {
+ this.tableStateManager = tableStateManager;
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransition.containsKey(regionInfo);
+ }
+
+ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
+ RegionState.State currentState = regionStateNode.getState();
+ // only consider default replica for availability
+ if (regionStateNode.getRegionInfo().getReplicaId() !=
RegionInfo.DEFAULT_REPLICA_ID) {
+ return;
+ }
+ // if region is merged or split it should not be in RIT list
+ if (
+ currentState == RegionState.State.SPLIT || currentState ==
RegionState.State.MERGED
+ || regionStateNode.getRegionInfo().isSplit()
+ ) {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ } else if
(!getExceptedRegionStates(regionStateNode).contains(currentState)) {
+ addRegionInTransition(regionStateNode);
+ } else {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ }
+ }
+
+ public void handleRegionDelete(RegionInfo regionInfo) {
+ removeRegionInTransition(regionInfo);
+ }
+
+ private List<RegionState.State> getExceptedRegionStates(RegionStateNode
regionStateNode) {
+ if (
+ tableStateManager.isTableState(regionStateNode.getTable(),
TableState.State.ENABLED,
+ TableState.State.ENABLING)
+ ) {
+ return ENABLE_TABLE_REGION_STATE;
+ } else {
+ return DISABLE_TABLE_REGION_STATE;
+ }
+ }
+
+ public void addRegionInTransition(final RegionStateNode regionStateNode) {
+ if (regionInTransition.putIfAbsent(regionStateNode.getRegionInfo(),
regionStateNode) == null) {
+ LOG.info("Added to RIT list " +
regionStateNode.getRegionInfo().getEncodedName());
+ }
+ }
+
+ public void removeRegionInTransition(final RegionInfo regionInfo) {
+ if (regionInTransition.remove(regionInfo) != null) {
+ LOG.info("Removed from RIT list " + regionInfo.getEncodedName());
+ }
+ }
+
+ public void clear() {
+ regionInTransition.clear();
Review Comment:
When the list is cleared this way there are no log lines indicating the
regioninfo was removed from the list. Is that a problem?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.assignment;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.TableStateManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+public class RegionInTransitionTracker {
+ private static final Logger LOG =
LoggerFactory.getLogger(RegionInTransitionTracker.class);
+
+ private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
+ List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
+
+ private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
List.of(RegionState.State.OPEN);
+
+ private final ConcurrentSkipListMap<RegionInfo, RegionStateNode>
regionInTransition =
+ new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
+ private final TableStateManager tableStateManager;
+
+ public RegionInTransitionTracker(TableStateManager tableStateManager) {
+ this.tableStateManager = tableStateManager;
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransition.containsKey(regionInfo);
+ }
+
+ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
+ RegionState.State currentState = regionStateNode.getState();
+ // only consider default replica for availability
+ if (regionStateNode.getRegionInfo().getReplicaId() !=
RegionInfo.DEFAULT_REPLICA_ID) {
+ return;
+ }
+ // if region is merged or split it should not be in RIT list
+ if (
+ currentState == RegionState.State.SPLIT || currentState ==
RegionState.State.MERGED
+ || regionStateNode.getRegionInfo().isSplit()
+ ) {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ } else if
(!getExceptedRegionStates(regionStateNode).contains(currentState)) {
+ addRegionInTransition(regionStateNode);
+ } else {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ }
+ }
+
+ public void handleRegionDelete(RegionInfo regionInfo) {
+ removeRegionInTransition(regionInfo);
+ }
+
+ private List<RegionState.State> getExceptedRegionStates(RegionStateNode
regionStateNode) {
+ if (
+ tableStateManager.isTableState(regionStateNode.getTable(),
TableState.State.ENABLED,
+ TableState.State.ENABLING)
+ ) {
+ return ENABLE_TABLE_REGION_STATE;
+ } else {
+ return DISABLE_TABLE_REGION_STATE;
+ }
+ }
+
+ public void addRegionInTransition(final RegionStateNode regionStateNode) {
+ if (regionInTransition.putIfAbsent(regionStateNode.getRegionInfo(),
regionStateNode) == null) {
+ LOG.info("Added to RIT list " +
regionStateNode.getRegionInfo().getEncodedName());
+ }
+ }
+
+ public void removeRegionInTransition(final RegionInfo regionInfo) {
+ if (regionInTransition.remove(regionInfo) != null) {
+ LOG.info("Removed from RIT list " + regionInfo.getEncodedName());
Review Comment:
Say why it was removed. Should this be DEBUG level?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.assignment;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.TableStateManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+public class RegionInTransitionTracker {
+ private static final Logger LOG =
LoggerFactory.getLogger(RegionInTransitionTracker.class);
+
+ private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
+ List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
+
+ private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
List.of(RegionState.State.OPEN);
+
+ private final ConcurrentSkipListMap<RegionInfo, RegionStateNode>
regionInTransition =
+ new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
+ private final TableStateManager tableStateManager;
+
+ public RegionInTransitionTracker(TableStateManager tableStateManager) {
+ this.tableStateManager = tableStateManager;
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransition.containsKey(regionInfo);
+ }
+
+ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
+ RegionState.State currentState = regionStateNode.getState();
+ // only consider default replica for availability
+ if (regionStateNode.getRegionInfo().getReplicaId() !=
RegionInfo.DEFAULT_REPLICA_ID) {
+ return;
+ }
+ // if region is merged or split it should not be in RIT list
+ if (
+ currentState == RegionState.State.SPLIT || currentState ==
RegionState.State.MERGED
+ || regionStateNode.getRegionInfo().isSplit()
+ ) {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ } else if
(!getExceptedRegionStates(regionStateNode).contains(currentState)) {
+ addRegionInTransition(regionStateNode);
+ } else {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ }
+ }
+
+ public void handleRegionDelete(RegionInfo regionInfo) {
+ removeRegionInTransition(regionInfo);
+ }
+
+ private List<RegionState.State> getExceptedRegionStates(RegionStateNode
regionStateNode) {
Review Comment:
I think this method should be renamed and the code should be documented with
comments. You are not really "excepting" regions.
You are basically waiting for the assignment state machine to complete and
reach the desired terminal state by checking the region's current state against
the table's state (ENABLED vs. DISABLED) to determine if the region is in the
terminal state. If not, the region is added to the RIT list; otherwise, it is
removed.
Your method naming and comments should communicate this theory of operation.
Related, there may be a minor race condition here. Consider if the table's
state is changed (e.g., from ENABLED to DISABLING) at the same time a region
for that table reports a state change. I think the tracker can momentarily use
a stale table state here. However, this is self-correcting. As the region
proceeds through its state transitions each subsequent call to the tracker will
re-evaluate its status, and it will eventually be removed from RIT correctly.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionInTransitionTracker.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master.assignment;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.TableStateManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+public class RegionInTransitionTracker {
+ private static final Logger LOG =
LoggerFactory.getLogger(RegionInTransitionTracker.class);
+
+ private final List<RegionState.State> DISABLE_TABLE_REGION_STATE =
+ List.of(RegionState.State.OFFLINE, RegionState.State.CLOSED);
+
+ private final List<RegionState.State> ENABLE_TABLE_REGION_STATE =
List.of(RegionState.State.OPEN);
+
+ private final ConcurrentSkipListMap<RegionInfo, RegionStateNode>
regionInTransition =
+ new ConcurrentSkipListMap<>(RegionInfo.COMPARATOR);
+
+ private final TableStateManager tableStateManager;
+
+ public RegionInTransitionTracker(TableStateManager tableStateManager) {
+ this.tableStateManager = tableStateManager;
+ }
+
+ public boolean isRegionInTransition(final RegionInfo regionInfo) {
+ return regionInTransition.containsKey(regionInfo);
+ }
+
+ public void handleRegionStateNodeOperation(RegionStateNode regionStateNode) {
+ RegionState.State currentState = regionStateNode.getState();
+ // only consider default replica for availability
+ if (regionStateNode.getRegionInfo().getReplicaId() !=
RegionInfo.DEFAULT_REPLICA_ID) {
+ return;
+ }
+ // if region is merged or split it should not be in RIT list
+ if (
+ currentState == RegionState.State.SPLIT || currentState ==
RegionState.State.MERGED
+ || regionStateNode.getRegionInfo().isSplit()
+ ) {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ } else if
(!getExceptedRegionStates(regionStateNode).contains(currentState)) {
+ addRegionInTransition(regionStateNode);
+ } else {
+ removeRegionInTransition(regionStateNode.getRegionInfo());
+ }
+ }
+
+ public void handleRegionDelete(RegionInfo regionInfo) {
+ removeRegionInTransition(regionInfo);
+ }
+
+ private List<RegionState.State> getExceptedRegionStates(RegionStateNode
regionStateNode) {
+ if (
+ tableStateManager.isTableState(regionStateNode.getTable(),
TableState.State.ENABLED,
+ TableState.State.ENABLING)
+ ) {
+ return ENABLE_TABLE_REGION_STATE;
+ } else {
+ return DISABLE_TABLE_REGION_STATE;
+ }
+ }
+
+ public void addRegionInTransition(final RegionStateNode regionStateNode) {
+ if (regionInTransition.putIfAbsent(regionStateNode.getRegionInfo(),
regionStateNode) == null) {
+ LOG.info("Added to RIT list " +
regionStateNode.getRegionInfo().getEncodedName());
Review Comment:
Say why it was added. Should this be DEBUG level?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
