HBASE-24015: Test for Assign and Unassign of Regions on RegionServer on failure (#1898)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Viraj Jasani <vjasani@apache.org>

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

@@ -123,7 +123,7 @@ public class TransitRegionStateProcedure
   public TransitRegionStateProcedure() {
   }
 
-  private void setInitalAndLastState() {
+  private void setInitialAndLastState() {
     switch (type) {
       case ASSIGN:
         initialState = RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE;
@@ -150,7 +150,7 @@ public class TransitRegionStateProcedure
     this.assignCandidate = assignCandidate;
     this.forceNewPlan = forceNewPlan;
     this.type = type;
-    setInitalAndLastState();
+    setInitialAndLastState();
 
     // when do reopen TRSP, let the rs know the targetServer so it can keep some info on close
     if (type == TransitionType.REOPEN) {
@@ -506,7 +506,7 @@ public class TransitRegionStateProcedure
     RegionStateTransitionStateData data =
       serializer.deserialize(RegionStateTransitionStateData.class);
     type = convert(data.getType());
-    setInitalAndLastState();
+    setInitialAndLastState();
     forceNewPlan = data.getForceNewPlan();
     if (data.hasAssignCandidate()) {
       assignCandidate = ProtobufUtil.toServerName(data.getAssignCandidate());

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java

@@ -94,9 +94,8 @@ public class CloseRegionHandler extends EventHandler {
   public void process() throws IOException {
     String name = regionInfo.getEncodedName();
     LOG.trace("Processing close of {}", name);
-    String encodedRegionName = regionInfo.getEncodedName();
     // Check that this region is being served here
-    HRegion region = (HRegion)rsServices.getRegion(encodedRegionName);
+    HRegion region = (HRegion)rsServices.getRegion(name);
     try {
       if (region == null) {
         LOG.warn("Received CLOSE for region {} but currently not serving - ignoring", name);

hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestExceptionInAssignRegion.java

@@ -0,0 +1,130 @@
+/**
+ * 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.Optional;
+import java.util.concurrent.CountDownLatch;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.coprocessor.RegionObserver;
+import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
+import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.JVMClusterUtil;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ MasterTests.class, MediumTests.class })
+public class TestExceptionInAssignRegion {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestExceptionInAssignRegion.class);
+
+  private static final HBaseTestingUtility UTIL = new HBaseTestingUtility();
+
+  private static final TableName TABLE_NAME = TableName.valueOf("test");
+
+  private static final CountDownLatch countDownLatch = new CountDownLatch(2);
+
+  private static final byte[] CF = Bytes.toBytes("cf");
+
+  @BeforeClass
+  public static void setUp() throws Exception {
+    UTIL.getConfiguration().setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
+      ThrowInOpenCP.class.getName());
+    UTIL.startMiniCluster(3);
+    UTIL.getAdmin().balancerSwitch(false, true);
+    UTIL.createTable(TABLE_NAME, CF);
+    UTIL.waitTableAvailable(TABLE_NAME);
+  }
+
+  @AfterClass
+  public static void tearDown() throws Exception {
+    UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testExceptionInAssignRegion() {
+    ProcedureExecutor procedureExecutor =
+      UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor();
+
+    JVMClusterUtil.RegionServerThread rsThread = null;
+    for (JVMClusterUtil.RegionServerThread t : UTIL.getMiniHBaseCluster()
+      .getRegionServerThreads()) {
+      if (!t.getRegionServer().getRegions(TABLE_NAME).isEmpty()) {
+        rsThread = t;
+        break;
+      }
+    }
+    // find the rs and hri of the table
+    HRegionServer rs = rsThread.getRegionServer();
+    RegionInfo hri = rs.getRegions(TABLE_NAME).get(0).getRegionInfo();
+    TransitRegionStateProcedure assignRegionProcedure = TransitRegionStateProcedure.move(
+      UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().getEnvironment(),
+      hri, null);
+    RegionStateNode regionNode = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager()
+      .getRegionStates().getOrCreateRegionStateNode(hri);
+    regionNode.setProcedure(assignRegionProcedure);
+    countDownLatch.countDown();
+    long prodId = procedureExecutor.submitProcedure(assignRegionProcedure);
+    ProcedureTestingUtility.waitProcedure(procedureExecutor, prodId);
+
+    Assert.assertEquals("Should be two RS since other is aborted", 2,
+      UTIL.getMiniHBaseCluster().getLiveRegionServerThreads().size());
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(0).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(1).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(2).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+  }
+
+  private HRegionServer getRegionServer(int index) {
+    return UTIL.getMiniHBaseCluster().getRegionServer(index);
+  }
+
+  public static class ThrowInOpenCP implements RegionCoprocessor, RegionObserver {
+    @Override public void preOpen(ObserverContext<RegionCoprocessorEnvironment> c) {
+      if (countDownLatch.getCount() == 1) {
+        // We want to throw exception only first time in move region call
+        // After that RS aborts and we don't want to throw in any other open region
+        countDownLatch.countDown();
+        throw new RuntimeException();
+      }
+    }
+
+    @Override
+    public Optional<RegionObserver> getRegionObserver() {
+      return Optional.of(this);
+    }
+  }
+}

hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestExceptionInUnassignedRegion.java

@@ -0,0 +1,124 @@
+/**
+ * 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.Optional;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.coprocessor.RegionObserver;
+import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
+import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.JVMClusterUtil;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ MasterTests.class, MediumTests.class })
+public class TestExceptionInUnassignedRegion {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestExceptionInUnassignedRegion.class);
+
+  private static final HBaseTestingUtility UTIL = new HBaseTestingUtility();
+
+  private static final TableName TABLE_NAME = TableName.valueOf("test");
+
+  private static final byte[] CF = Bytes.toBytes("cf");
+
+  @BeforeClass
+  public static void setUp() throws Exception {
+    UTIL.getConfiguration().setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
+      ThrowInCloseCP.class.getName());
+    UTIL.startMiniCluster(3);
+    UTIL.getAdmin().balancerSwitch(false, true);
+    UTIL.createTable(TABLE_NAME, CF);
+    UTIL.waitTableAvailable(TABLE_NAME);
+  }
+
+  @AfterClass
+  public static void tearDown() throws Exception {
+    UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testExceptionInUnassignRegion() {
+    ProcedureExecutor procedureExecutor =
+      UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor();
+
+    JVMClusterUtil.RegionServerThread rsThread = null;
+    for (JVMClusterUtil.RegionServerThread t : UTIL.getMiniHBaseCluster()
+      .getRegionServerThreads()) {
+      if (!t.getRegionServer().getRegions(TABLE_NAME).isEmpty()) {
+        rsThread = t;
+        break;
+      }
+    }
+    // find the rs and hri of the table
+    HRegionServer rs = rsThread.getRegionServer();
+    RegionInfo hri = rs.getRegions(TABLE_NAME).get(0).getRegionInfo();
+    TransitRegionStateProcedure moveRegionProcedure = TransitRegionStateProcedure.reopen(
+      UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().getEnvironment(), hri);
+    RegionStateNode regionNode = UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager()
+      .getRegionStates().getOrCreateRegionStateNode(hri);
+    regionNode.setProcedure(moveRegionProcedure);
+    long prodId = procedureExecutor.submitProcedure(moveRegionProcedure);
+    ProcedureTestingUtility.waitProcedure(procedureExecutor, prodId);
+
+    Assert.assertEquals("Should be two RS since other is aborted", 2,
+      UTIL.getMiniHBaseCluster().getLiveRegionServerThreads().size());
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(0).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(1).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+    Assert.assertNull("RIT Map doesn't have correct value",
+      getRegionServer(2).getRegionsInTransitionInRS().get(hri.getEncodedNameAsBytes()));
+  }
+
+  private HRegionServer getRegionServer(int index) {
+    return UTIL.getMiniHBaseCluster().getRegionServer(index);
+  }
+
+  public static class ThrowInCloseCP implements RegionCoprocessor, RegionObserver {
+
+    @Override
+    public void preClose(ObserverContext<RegionCoprocessorEnvironment> c, boolean abortRequested) {
+      if (!c.getEnvironment().getRegion().getRegionInfo().getTable().isSystemTable()) {
+        throw new RuntimeException();
+      }
+    }
+
+    @Override
+    public Optional<RegionObserver> getRegionObserver() {
+      return Optional.of(this);
+    }
+  }
+}