HBASE-29251 Procedure gets stuck if the procedure state cannot be persisted

hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegion.java

@@ -118,6 +118,11 @@ public final class MasterRegion {
 
   private MasterRegionWALRoller walRoller;
 
+  /**
+   * This is only for test purpose.
+   */
+  private boolean updateFailForTest = false;
+
   private MasterRegion(Server server, HRegion region, WALFactory walFactory,
     MasterRegionFlusherAndCompactor flusherAndCompactor, MasterRegionWALRoller walRoller) {
     this.server = server;
@@ -143,13 +148,23 @@ private void shutdownWAL() {
     }
   }
 
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+      allowedOnPath = ".*/src/test/.*")
+  public void setTestFailure() {
+    this.updateFailForTest = true;
+  }
+
   public void update(UpdateMasterRegion action) throws IOException {
     try {
+      if (updateFailForTest) {
+        // test for HBASE-29251
+        throw new IOException("Update failed");
+      }
       action.update(region);
       flusherAndCompactor.onUpdate();
-    } catch (WALSyncTimeoutIOException e) {
-      LOG.error(HBaseMarkers.FATAL, "WAL sync timeout. Aborting server.");
-      server.abort("WAL sync timeout", e);
+    } catch (IOException e) {
+      LOG.error(HBaseMarkers.FATAL, "MasterRegion mutation is not successful. Aborting server.");
+      server.abort("MasterRegion mutation is not successful", e);
       throw e;
     }
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMasterRegionMutation.java

@@ -0,0 +1,158 @@
+/*
+ * 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.util;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.SingleProcessHBaseCluster;
+import org.apache.hadoop.hbase.StartTestingClusterOption;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.master.hbck.HbckChore;
+import org.apache.hadoop.hbase.master.hbck.HbckReport;
+import org.apache.hadoop.hbase.master.region.MasterRegion;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.testclassification.MiscTests;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos;
+
+@Category({ MiscTests.class, LargeTests.class })
+public class TestMasterRegionMutation {
+
+  private static final Logger LOG = LoggerFactory.getLogger(TestMasterRegionMutation.class);
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestMasterRegionMutation.class);
+
+  @Rule
+  public TestName name = new TestName();
+
+  private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+  private static ServerName rs0;
+
+  @BeforeClass
+  public static void setUpBeforeClass() throws Exception {
+    StartTestingClusterOption.Builder builder = StartTestingClusterOption.builder();
+    builder.numMasters(2).numRegionServers(3);
+    TEST_UTIL.startMiniCluster(builder.build());
+    SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+    rs0 = cluster.getRegionServer(0).getServerName();
+    TEST_UTIL.getAdmin().balancerSwitch(false, true);
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  @Before
+  public void setUp() throws Exception {
+    final TableName tableName = TableName.valueOf(name.getMethodName());
+    TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build();
+    int startKey = 0;
+    int endKey = 80000;
+    TEST_UTIL.getAdmin().createTable(tableDesc, Bytes.toBytes(startKey), Bytes.toBytes(endKey), 9);
+  }
+
+  @Test
+  public void testMasterRegionMutations() throws Exception {
+    HbckChore hbckChore = new HbckChore(TEST_UTIL.getHBaseCluster().getMaster());
+    SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+
+    HRegionServer hRegionServer0 = cluster.getRegionServer(0);
+    HRegionServer hRegionServer1 = cluster.getRegionServer(1);
+    HRegionServer hRegionServer2 = cluster.getRegionServer(2);
+    int numRegions0 = hRegionServer0.getNumberOfOnlineRegions();
+    int numRegions1 = hRegionServer1.getNumberOfOnlineRegions();
+    int numRegions2 = hRegionServer2.getNumberOfOnlineRegions();
+
+    hbckChore.choreForTesting();
+    HbckReport hbckReport = hbckChore.getLastReport();
+    Assert.assertEquals(0, hbckReport.getInconsistentRegions().size());
+    Assert.assertEquals(0, hbckReport.getOrphanRegionsOnFS().size());
+    Assert.assertEquals(0, hbckReport.getOrphanRegionsOnRS().size());
+
+    MasterRegion masterRegion = cluster.getMaster().getMasterRegion();
+    masterRegion.setTestFailure();
+
+    // move one region from server 1 to server 0
+    TEST_UTIL.getAdmin()
+      .move(hRegionServer1.getRegions().get(0).getRegionInfo().getEncodedNameAsBytes(), rs0);
+
+    // move one region from server 2 to server 0
+    TEST_UTIL.getAdmin()
+      .move(hRegionServer2.getRegions().get(0).getRegionInfo().getEncodedNameAsBytes(), rs0);
+
+    HMaster master = TEST_UTIL.getHBaseCluster().getMaster();
+
+    // Ensure:
+    // 1. num of regions before and after master abort remain same
+    // 2. all procedures are successfully completed
+    TEST_UTIL.waitFor(5000, 1000, () -> {
+      LOG.info("numRegions0: {} , numRegions1: {} , numRegions2: {}", numRegions0, numRegions1,
+        numRegions2);
+      LOG.info("Online regions - server0 : {} , server1: {} , server2: {}",
+        cluster.getRegionServer(0).getNumberOfOnlineRegions(),
+        cluster.getRegionServer(1).getNumberOfOnlineRegions(),
+        cluster.getRegionServer(2).getNumberOfOnlineRegions());
+      LOG.info("Num of successfully completed procedures: {} , num of all procedures: {}",
+        master.getMasterProcedureExecutor().getProcedures().stream()
+          .filter(masterProcedureEnvProcedure -> masterProcedureEnvProcedure.getState()
+              == ProcedureProtos.ProcedureState.SUCCESS)
+          .count(),
+        master.getMasterProcedureExecutor().getProcedures().size());
+      return (numRegions0 + numRegions1 + numRegions2)
+          == (cluster.getRegionServer(0).getNumberOfOnlineRegions()
+            + cluster.getRegionServer(1).getNumberOfOnlineRegions()
+            + cluster.getRegionServer(2).getNumberOfOnlineRegions())
+        && master.getMasterProcedureExecutor().getProcedures().stream()
+          .filter(masterProcedureEnvProcedure -> masterProcedureEnvProcedure.getState()
+              == ProcedureProtos.ProcedureState.SUCCESS)
+          .count() == master.getMasterProcedureExecutor().getProcedures().size();
+    });
+
+    // Ensure we have no inconsistent regions
+    TEST_UTIL.waitFor(5000, 1000, () -> {
+      HbckChore hbck = new HbckChore(TEST_UTIL.getHBaseCluster().getMaster());
+      hbck.choreForTesting();
+      HbckReport report = hbck.getLastReport();
+      return report.getInconsistentRegions().isEmpty() && report.getOrphanRegionsOnFS().isEmpty()
+        && report.getOrphanRegionsOnRS().isEmpty();
+    });
+
+  }
+
+}