HBASE-26462 Should persist restoreAcl flag in the procedure state for CloneSnapshotProcedure and RestoreSnapshotProcedure (#3921)

Signed-off-by: Yu Li <liyu@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>

hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto

@@ -207,6 +207,7 @@ message CloneSnapshotStateData {
   required TableSchema table_schema = 3;
   repeated RegionInfo region_info = 4;
   repeated RestoreParentToChildRegionsPair parent_to_child_regions_pair_list = 5;
+  optional bool restore_acl = 6;
 }
 
 enum RestoreSnapshotState {
@@ -225,6 +226,7 @@ message RestoreSnapshotStateData {
   repeated RegionInfo region_info_for_remove = 5;
   repeated RegionInfo region_info_for_add = 6;
   repeated RestoreParentToChildRegionsPair parent_to_child_regions_pair_list = 7;
+  optional bool restore_acl = 8;
 }
 
 enum DispatchMergingRegionsState {

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hbase.master.procedure;
 
+import com.google.errorprone.annotations.RestrictedApi;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -271,6 +272,8 @@ public class CloneSnapshotProcedure
         .setUserInfo(MasterProcedureUtil.toProtoUserInfo(getUser()))
         .setSnapshot(this.snapshot)
         .setTableSchema(ProtobufUtil.toTableSchema(tableDescriptor));
+
+    cloneSnapshotMsg.setRestoreAcl(restoreAcl);
     if (newRegions != null) {
       for (RegionInfo hri: newRegions) {
         cloneSnapshotMsg.addRegionInfo(ProtobufUtil.toRegionInfo(hri));
@@ -303,6 +306,9 @@ public class CloneSnapshotProcedure
     setUser(MasterProcedureUtil.toUserInfo(cloneSnapshotMsg.getUserInfo()));
     snapshot = cloneSnapshotMsg.getSnapshot();
     tableDescriptor = ProtobufUtil.toTableDescriptor(cloneSnapshotMsg.getTableSchema());
+    if (cloneSnapshotMsg.hasRestoreAcl()) {
+      restoreAcl = cloneSnapshotMsg.getRestoreAcl();
+    }
     if (cloneSnapshotMsg.getRegionInfoCount() == 0) {
       newRegions = null;
     } else {
@@ -521,4 +527,13 @@ public class CloneSnapshotProcedure
     metaChanges.updateMetaParentRegions(env.getMasterServices().getConnection(), newRegions);
   }
 
+  /**
+   * Exposed for Testing: HBASE-26462
+   */
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+    allowedOnPath = ".*/src/test/.*")
+  public boolean getRestoreAcl() {
+    return restoreAcl;
+  }
+
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hbase.master.procedure;
 
+import com.google.errorprone.annotations.RestrictedApi;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -271,6 +272,7 @@ public class RestoreSnapshotProcedure
         restoreSnapshotMsg.addParentToChildRegionsPairList (parentToChildrenPair);
       }
     }
+    restoreSnapshotMsg.setRestoreAcl(restoreAcl);
     serializer.serialize(restoreSnapshotMsg.build());
   }
 
@@ -320,6 +322,9 @@ public class RestoreSnapshotProcedure
             parentToChildrenPair.getChild2RegionName()));
       }
     }
+    if (restoreSnapshotMsg.hasRestoreAcl()) {
+      restoreAcl = restoreSnapshotMsg.getRestoreAcl();
+    }
   }
 
   /**
@@ -545,4 +550,13 @@ public class RestoreSnapshotProcedure
         env.getMasterServices().getConfiguration());
     }
   }
+
+  /**
+   * Exposed for Testing: HBASE-26462
+   */
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+    allowedOnPath = ".*/src/test/.*")
+  public boolean getRestoreAcl() {
+    return restoreAcl;
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.master.procedure;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.util.List;
@@ -161,6 +162,31 @@ public class TestCloneSnapshotProcedure extends TestTableDDLProcedureBase {
       clonedTableName);
   }
 
+  @Test
+  public void testRecoverWithRestoreAclFlag() throws Exception {
+    // This test is to solve the problems mentioned in HBASE-26462,
+    // this needs to simulate the case of CloneSnapshotProcedure failure and recovery,
+    // and verify whether 'restoreAcl' flag can obtain the correct value.
+
+    final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+    final TableName clonedTableName = TableName.valueOf("testRecoverWithRestoreAclFlag");
+    final TableDescriptor htd = createTableDescriptor(clonedTableName, CF);
+
+    SnapshotProtos.SnapshotDescription snapshotDesc = getSnapshot();
+    ProcedureTestingUtility.setKillIfHasParent(procExec, false);
+    ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true);
+
+    // Start the Clone snapshot procedure (with restoreAcl 'true') && kill the executor
+    long procId = procExec.submitProcedure(
+      new CloneSnapshotProcedure(procExec.getEnvironment(), htd, snapshotDesc, true));
+
+    MasterProcedureTestingUtility.testRecoveryAndDoubleExecution(procExec, procId);
+
+    CloneSnapshotProcedure result = (CloneSnapshotProcedure)procExec.getResult(procId);
+    // check whether the 'restoreAcl' flag is true after deserialization from Pb.
+    assertEquals(true, result.getRestoreAcl());
+  }
+
   @Test
   public void testRollbackAndDoubleExecution() throws Exception {
     final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java

@@ -210,6 +210,25 @@ public class TestRestoreSnapshotProcedure extends TestTableDDLProcedureBase {
     validateSnapshotRestore();
   }
 
+  @Test
+  public void testRecoverWithRestoreAclFlag() throws Exception {
+    // This test is to solve the problems mentioned in HBASE-26462,
+    // this needs to simulate the case of RestoreSnapshotProcedure failure and recovery,
+    // and verify whether 'restoreAcl' flag can obtain the correct value.
+
+    final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+    ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true);
+
+    // Start the Restore snapshot procedure (with restoreAcl 'true') && kill the executor
+    long procId = procExec.submitProcedure(
+      new RestoreSnapshotProcedure(procExec.getEnvironment(), snapshotHTD, snapshot, true));
+    MasterProcedureTestingUtility.testRecoveryAndDoubleExecution(procExec, procId);
+
+    RestoreSnapshotProcedure result = (RestoreSnapshotProcedure)procExec.getResult(procId);
+    // check whether the restoreAcl flag is true after deserialization from Pb.
+    assertEquals(true, result.getRestoreAcl());
+  }
+
   private void validateSnapshotRestore() throws IOException {
     try {
       UTIL.getAdmin().enableTable(snapshotTableName);