Skip to content

HBASE-29190 Allow disabling regions in FAILED_OPEN state #6797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
junegunn wants to merge 7 commits into
base: master
Choose a base branch
from
+114 −5
Open

HBASE-29190 Allow disabling regions in FAILED_OPEN state #6797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d9ddd43
Add test case for disabling table with FAILED_OPEN regions
junegunn Mar 13, 2025
42715b6
Allow disabling FAILED_OPEN regions
junegunn Feb 12, 2025
cab6eb4
Remove unnecessary error handler
junegunn May 23, 2025
dd8716f
Add more steps to the test case (disable, fix, and re-enable)
junegunn May 23, 2025
718156e
Use two separate ProcedureFutureUtil.suspendIfNecessary calls
junegunn May 24, 2025
8bd00b5
Revert the previous change
junegunn May 24, 2025
c8ec640
Merge remote-tracking branch 'origin/master' into HBASE-29190
junegunn Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2087,7 +2087,7 @@ public RegionInfo getRegionInfo(final String encodedRegionName) {
State.OPEN // Retrying
};

private static final State[] STATES_EXPECTED_ON_CLOSING = { State.OPEN, // Normal case
static final State[] STATES_EXPECTED_ON_CLOSING = { State.OPEN, // Normal case
State.CLOSING, // Retrying
State.SPLITTING, // Offline the split parent
State.MERGING // Offline the merge parents
Expand Down
30 changes: 26 additions & 4 deletions .../src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_SPLIT_KEY;
import static org.apache.hadoop.hbase.master.LoadBalancer.BOGUS_SERVER_NAME;
import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT;
import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.STATES_EXPECTED_ON_CLOSING;

import edu.umd.cs.findbugs.annotations.Nullable;
import java.io.IOException;
Expand Down Expand Up @@ -371,6 +372,13 @@ private Flow confirmOpened(MasterProcedureEnv env, RegionStateNode regionNode)
}

private void closeRegionAfterUpdatingMeta(MasterProcedureEnv env, RegionStateNode regionNode) {
// Absence of location indicates that this region was in FAILED_OPEN state.
// This happens when disabling a table with regions in FAILED_OPEN state.
if (regionNode.getRegionLocation() == null) {
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_CONFIRM_CLOSED);
return;
}

LOG.debug("Close region: isSplit: {}: evictOnSplit: {}: evictOnClose: {}", isSplit,
env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_SPLIT_KEY, DEFAULT_EVICT_ON_SPLIT),
evictCache);
Expand All @@ -391,10 +399,24 @@ private void closeRegion(MasterProcedureEnv env, RegionStateNode regionNode)
) {
return;
}
if (regionNode.isInState(State.OPEN, State.CLOSING, State.MERGING, State.SPLITTING)) {
// this is the normal case
ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture,
env.getAssignmentManager().regionClosing(regionNode), env,

CompletableFuture<Void> future = null;
if (regionNode.isInState(STATES_EXPECTED_ON_CLOSING)) {
// This is the normal case
future = env.getAssignmentManager().regionClosing(regionNode);
} else if (regionNode.setState(State.CLOSED, State.FAILED_OPEN)) {
// If a region was in FAILED_OPEN state, it was not OPEN and effectively CLOSED.
// So we should not try to close it again. We just need to update the state to CLOSED.

// Remove the region from RIT list to prevent periodic "RITs over threshold" messages.
final AssignmentManager am = env.getAssignmentManager();
am.getRegionStates().removeFromFailedOpen(regionNode.getRegionInfo());

// Persistent CLOSED state to meta and proceed to the next state.
future = am.getRegionStateStore().updateRegionLocation(regionNode);
Copy link
Contributor

@Apache9 Apache9 May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to set the state to CLOSED when the state is FAILED_OPEN? And if the state is CLOSED, do we still need to call udpateRegionLocation?

Copy link
Member Author

@junegunn junegunn May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. Before I answer the questions, please take a look at dd8716f. I added a few more steps so that the intention of this change is clearer.

We do not need to set the state to CLOSED when the state is FAILED_OPEN?

So the question is: "Is regionNode.setState(State.CLOSED, State.FAILED_OPEN) really necessary? Can we just check regionNode.isInState(State.FAILED_OPEN) here?" Am I right?

If we don't change the state,

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
index 6b5aaf6975..215e1245ed 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
@@ -404,11 +404,13 @@ public class TransitRegionStateProcedure
     if (regionNode.isInState(STATES_EXPECTED_ON_CLOSING)) {
       // This is the normal case
       future = env.getAssignmentManager().regionClosing(regionNode);
-    } else if (regionNode.setState(State.CLOSED, State.FAILED_OPEN)) {
-      // FAILED_OPEN doesn't need further transition, immediately mark the region as closed
+    } else if (regionNode.isInState(State.FAILED_OPEN)) {
+      // Remove the region from RIT list to suppress periodic "RITs over threshold" messages
       AssignmentManager am = env.getAssignmentManager();
       am.getRegionStates().removeFromFailedOpen(regionNode.getRegionInfo());
-      future = am.getRegionStateStore().updateRegionLocation(regionNode);
+
+      // FAILED_OPEN doesn't need further transition
+      future = CompletableFuture.allOf();
     }
     if (future != null) {
       ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture, future, env,

we will run into an assertion error in confirmClosed:

hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java

Lines 423 to 449 in cab6eb4

private Flow confirmClosed(MasterProcedureEnv env, RegionStateNode regionNode)
throws IOException {
if (regionNode.isInState(State.CLOSED)) {
retryCounter = null;
if (lastState == RegionStateTransitionState.REGION_STATE_TRANSITION_CONFIRM_CLOSED) {
// we are the last state, finish
regionNode.unsetProcedure(this);
return Flow.NO_MORE_STATE;
}
// This means we need to open the region again, should be a move or reopen
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return Flow.HAS_MORE_STATE;
}
if (regionNode.isInState(State.CLOSING)) {
// This is possible, think the target RS crashes and restarts immediately, the close region
// operation will return a NotServingRegionException soon, we can only recover after SCP takes
// care of this RS. So here we throw an IOException to let upper layer to retry with backoff.
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_CLOSE);
throw new HBaseIOException("Failed to close region");
}
// abnormally closed, need to reopen it, no matter what is the last state, see the comment in
// confirmOpened for more details that why we need to reopen the region first even if we just
// want to close it.
// The only exception is for non-default replica, where we do not need to deal with recovered
// edits. Notice that the region will remain in ABNORMALLY_CLOSED state, the upper layer need to
// deal with this state. For non-default replica, this is usually the same with CLOSED.
assert regionNode.isInState(State.ABNORMALLY_CLOSED);

Because FAILED_OPEN is not covered in the conditions. If we add FAILED_OPEN here like so:

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
index 6b5aaf6975..7cf0941f20 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
@@ -422,7 +424,7 @@ public class TransitRegionStateProcedure
 
   private Flow confirmClosed(MasterProcedureEnv env, RegionStateNode regionNode)
     throws IOException {
-    if (regionNode.isInState(State.CLOSED)) {
+    if (regionNode.isInState(State.CLOSED, State.FAILED_OPEN)) {
       retryCounter = null;
       if (lastState == RegionStateTransitionState.REGION_STATE_TRANSITION_CONFIRM_CLOSED) {
         // we are the last state, finish

We can avoid the assertion error, but CloseTableRegionsProcedure will not finish and endlessly retry:

There are still 1 unclosed region(s) for closing regions of table testDisableFailedOpenRegions when executing CloseTableRegionsProcedure, continue...

So we also need to change the implementation of numberOfUnclosedRegions to account for FAILED_OPEN regions.

hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java

Lines 1147 to 1163 in 42715b6

private int numberOfUnclosedRegions(TableName tableName,
Function<RegionStateNode, Boolean> shouldSubmit) {
int unclosed = 0;
for (RegionStateNode regionNode : regionStates.getTableRegionStateNodes(tableName)) {
regionNode.lock();
try {
if (shouldSubmit.apply(regionNode)) {
if (!regionNode.isInState(State.OFFLINE, State.CLOSED, State.SPLIT)) {
unclosed++;
}
}
} finally {
regionNode.unlock();
}
}
return unclosed;
}

But I felt this was getting too complicated, and it's simpler to just change the state to CLOSED.

Copy link
Member Author

@junegunn junegunn May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if the state is CLOSED, do we still need to call udpateRegionLocation?

I called the method to persist the in-memory change of regionNode to the meta table. Would it be clearer if I call persistToMeta instead? The result is roughly the same.

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
index 6b5aaf6975..69c9d1ffca 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
@@ -408,7 +408,7 @@ public class TransitRegionStateProcedure
       // FAILED_OPEN doesn't need further transition, immediately mark the region as closed
       AssignmentManager am = env.getAssignmentManager();
       am.getRegionStates().removeFromFailedOpen(regionNode.getRegionInfo());
-      future = am.getRegionStateStore().updateRegionLocation(regionNode);
+      future = am.persistToMeta(regionNode);
     }
     if (future != null) {
       ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture, future, env,

If the question is about if it's necessary to persist the changed state to meta, it looks like it's not necessary in this case, though we have to change an assertion in the test code.

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
index 6b5aaf6975..6b7989dd58 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
@@ -408,7 +408,7 @@ public class TransitRegionStateProcedure
       // FAILED_OPEN doesn't need further transition, immediately mark the region as closed
       AssignmentManager am = env.getAssignmentManager();
       am.getRegionStates().removeFromFailedOpen(regionNode.getRegionInfo());
-      future = am.getRegionStateStore().updateRegionLocation(regionNode);
+      future = CompletableFuture.allOf();
     }
     if (future != null) {
       ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture, future, env,
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestTransitRegionStateProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestTransitRegionStateProcedure.java
index 074e7e730c..392993280b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestTransitRegionStateProcedure.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestTransitRegionStateProcedure.java
@@ -250,8 +250,8 @@ public class TestTransitRegionStateProcedure {
     // The number of RITs should be 0 after disabling the table
     assertEquals(0, getTotalRITs());
 
-    // The regions are now in "CLOSED" state
-    assertEquals(Collections.singleton("CLOSED"), getRegionStates());
+    // The regions are still in "FAILED_OPEN" state
+    assertEquals(Collections.singleton("FAILED_OPEN"), getRegionStates());
 
     // Fix the error in the table descriptor
     tdb = TableDescriptorBuilder.newBuilder(td);

However, I think we should try to keep the in-memory state and persistent meta state synchronized to avoid confusion and any potential issues that might arise.

Copy link
Contributor

@Apache9 Apache9 May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, in the if condition you call setState, I misread the code, I thought it is isInState.

Then the logic is fine. But better to add more comments to say why here we do not need to treat CLOSED state specially.

And I suggest that we use two ProcedureFutureUtil.suspendIfNecessary calls for these two conditions, so we do not need to add extra check in closeRegionAfterUpdatingMeta, since for the region in FAILED_OPEN state, the only action after updating meta is to change the state.

Copy link
Member Author

@junegunn junegunn May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I suggest that we use two ProcedureFutureUtil.suspendIfNecessary calls for these two conditions, so we do not need to add extra check in closeRegionAfterUpdatingMeta

Ahh.. I remembered why I organized the code this way. This was not super obvious.

When you disable the table with "FAILED_OPEN" regions,

  1. DisableTableProcedure creates a CloseTableRegionsProcedure,
  2. It results in TRSP with REGION_STATE_TRANSITION_CLOSE as the initial state

    hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java

    Lines 157 to 160 in cab6eb4

    case UNASSIGN:
    initialState = RegionStateTransitionState.REGION_STATE_TRANSITION_CLOSE;
    lastState = RegionStateTransitionState.REGION_STATE_TRANSITION_CONFIRM_CLOSED;
    break;
  3. Because of the initial condition, it calls TRSP#closeRegion,

    hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java

    Lines 516 to 518 in cab6eb4

    case REGION_STATE_TRANSITION_CLOSE:
    closeRegion(env, regionNode);
    return Flow.HAS_MORE_STATE;
  4. which in turn calls closeRegionAfterUpdatingMeta

    hbase/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java

    Lines 394 to 401 in cab6eb4

    private void closeRegion(MasterProcedureEnv env, RegionStateNode regionNode)
    throws IOException, ProcedureSuspendedException {
    if (
    ProcedureFutureUtil.checkFuture(this, this::getFuture, this::setFuture,
    () -> closeRegionAfterUpdatingMeta(env, regionNode))
    ) {
    return;
    }
  5. So we're entering closeRegionAfterUpdatingMeta for a FAILED_OPEN region and HBase creates a CloseRegionProcedure which is exactly what we tried to avoid.
  6. CloseRegionProcedure is a subclass of RegionRemoteProcedureBase which requires targetServer, but a FAILED_OPEN region lacks it, and the procedure fails and hangs. 
    2025-05-23T19:17:19,176 WARN  [PEWorker-1 {}] procedure2.ProcedureExecutor$WorkerThread(2184): Worker terminating UNNATURALLY null
java.lang.NullPointerException: null
    at org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos$RegionRemoteProcedureBaseStateData$Builder.setTargetServer(MasterProcedureProtos.java:54339) ~[classes/:?]
    at org.apache.hadoop.hbase.master.assignment.RegionRemoteProcedureBase.serializeStateData(RegionRemoteProcedureBase.java:382) ~[classes/:?]
    at org.apache.hadoop.hbase.master.assignment.CloseRegionProcedure.serializeStateData(CloseRegionProcedure.java:74) ~[classes/:?]

So that explains why I had to put the check at the start of closeRegionAfterUpdatingMeta.

Copy link
Member Author

@junegunn junegunn May 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it looks like my analysis was not entirely correct, because at step 4, the initial call to checkFuture { closeRegionAfterUpdatingMeta } will not actually trigger closeRegionAfterUpdatingMeta. But the suspension of the procedure in suspendIfNecessary is what triggers multiple closeRegion calls which leads to a call to closeRegionAfterUpdatingMeta.

Copy link
Member Author

@junegunn junegunn May 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I suggest that we use two ProcedureFutureUtil.suspendIfNecessary calls for these two conditions,

Addressed that in 718156e. I also added more comments.

so we do not need to add extra check in closeRegionAfterUpdatingMeta

The extra check is still there for the reason mentioned above.

Copy link
Member Author

@junegunn junegunn May 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed that in 718156e. I also added more comments.

Apologies for the noise. I reverted the change after realizing that ProcedureFutureUtil.suspendIfNecessary does not guarantee execution of actionAfterDone when its future is suspended. So we may get unpredictable behavior if actionAfterDone of the previous checkFuture and that of suspendIfNecessary are not identical.

  1. checkFuture(null) { closeRegionAfterUpdatingMeta }
  2. suspendIfNecessary(updateRegionLocation) { setNextState }
    1. suspended
      1. closeRegion called again
      2. checkFuture(updateRegionLocation) { closeRegionAfterUpdatingMeta }
      3. closeRegionAfterUpdatingMeta
    2. not suspended
      1. setNextState

So with the latest commit, closeRegionAfterUpdatingMeta serves as the only terminal point of the procedure, regardless of the previous state of the region or whether the future was suspended or not.

}
if (future != null) {
ProcedureFutureUtil.suspendIfNecessary(this, this::setFuture, future, env,
() -> closeRegionAfterUpdatingMeta(env, regionNode));
} else {
forceNewPlan = true;
Expand Down
87 changes: 87 additions & 0 deletions .../test/java/org/apache/hadoop/hbase/master/assignment/TestTransitRegionStateProcedure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,42 @@
*/
package org.apache.hadoop.hbase.master.assignment;

import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.ASSIGN_MAX_ATTEMPTS;
import static org.apache.hadoop.hbase.master.assignment.RegionStateStore.getStateColumn;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.CellComparator;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtil;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.client.TableState;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureTestingUtility;
import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
import org.apache.hadoop.hbase.regionserver.DefaultStoreEngine;
import org.apache.hadoop.hbase.regionserver.HRegion;
import org.apache.hadoop.hbase.regionserver.HRegionServer;
import org.apache.hadoop.hbase.regionserver.HStore;
import org.apache.hadoop.hbase.testclassification.MasterTests;
import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.util.Bytes;
Expand Down Expand Up @@ -66,6 +85,7 @@ public class TestTransitRegionStateProcedure {
@BeforeClass
public static void setUpBeforeClass() throws Exception {
UTIL.getConfiguration().setInt(MasterProcedureConstants.MASTER_PROCEDURE_THREADS, 1);
UTIL.getConfiguration().setInt(ASSIGN_MAX_ATTEMPTS, 1);
UTIL.startMiniCluster(3);
UTIL.getAdmin().balancerSwitch(false, true);
}
Expand Down Expand Up @@ -168,4 +188,71 @@ public void testRecoveryAndDoubleExecutionUnassignAndAssign() throws Exception {
// confirm that the region is successfully opened
assertTrue(openSeqNum2 > openSeqNum);
}

private static class BuggyStoreEngine extends DefaultStoreEngine {
@Override
protected void createComponents(Configuration conf, HStore store, CellComparator comparator)
throws IOException {
throw new IOException("I'm broken");
}
}

private Set<String> getRegionStates() throws IOException {
List<RegionInfo> regions = MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName);
Set<String> regionStates = new HashSet<>();
for (RegionInfo region : regions) {
Result result = MetaTableAccessor.getRegionResult(UTIL.getConnection(), region);
String state = Bytes.toString(
result.getValue(HConstants.CATALOG_FAMILY, getStateColumn(region.getReplicaId())));
regionStates.add(state);
}
return regionStates;
}

private static int getTotalRITs() throws IOException {
final AssignmentManager am = UTIL.getHBaseCluster().getMaster().getAssignmentManager();
return am.computeRegionInTransitionStat().getTotalRITs();
}

@Test
public void testDisableFailedOpenRegions() throws Exception {
// Perform a faulty modification to put regions into FAILED_OPEN state
Admin admin = UTIL.getAdmin();
TableDescriptor td = admin.getDescriptor(tableName);
TableDescriptorBuilder tdb = TableDescriptorBuilder.newBuilder(td);
tdb.setValue("hbase.hstore.engine.class", BuggyStoreEngine.class.getName());
try {
admin.modifyTable(tdb.build());
fail("Should fail to modify the table");
} catch (IOException e) {
// expected
}

// Table is still "ENABLED"
assertEquals(TableState.State.ENABLED,
MetaTableAccessor.getTableState(UTIL.getConnection(), tableName).getState());

// But the regions are in "FAILED_OPEN" state
assertEquals(Collections.singleton("FAILED_OPEN"), getRegionStates());

// We should be able to disable the table
assertNotEquals(0, getTotalRITs());
UTIL.getAdmin().disableTable(tableName);

// The number of RITs should be 0 after disabling the table
assertEquals(0, getTotalRITs());

// The regions are now in "CLOSED" state
assertEquals(Collections.singleton("CLOSED"), getRegionStates());

// Fix the error in the table descriptor
tdb = TableDescriptorBuilder.newBuilder(td);
tdb.setValue("hbase.hstore.engine.class", DefaultStoreEngine.class.getName());
admin.modifyTable(tdb.build());

// We can then re-enable the table
UTIL.getAdmin().enableTable(tableName);
assertEquals(Collections.singleton("OPEN"), getRegionStates());
assertEquals(0, getTotalRITs());
}
}