You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2021/12/06 15:21:26 UTC

[hbase] branch branch-2 updated: HBASE-26462 Should persist restoreAcl flag in the procedure state for CloneSnapshotProcedure and RestoreSnapshotProcedure (#3921)

This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new ec52519  HBASE-26462 Should persist restoreAcl flag in the procedure state for CloneSnapshotProcedure and RestoreSnapshotProcedure (#3921)
ec52519 is described below

commit ec52519f7c4722843ba63a747efd1d51ea36df96
Author: LiangJun He <20...@163.com>
AuthorDate: Mon Dec 6 22:58:38 2021 +0800

    HBASE-26462 Should persist restoreAcl flag in the procedure state for CloneSnapshotProcedure and RestoreSnapshotProcedure (#3921)
    
    Signed-off-by: Yu Li <li...@apache.org>
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../src/main/protobuf/MasterProcedure.proto        |  2 ++
 .../master/procedure/CloneSnapshotProcedure.java   | 15 +++++++++++++
 .../master/procedure/RestoreSnapshotProcedure.java | 14 ++++++++++++
 .../procedure/TestCloneSnapshotProcedure.java      | 26 ++++++++++++++++++++++
 .../procedure/TestRestoreSnapshotProcedure.java    | 19 ++++++++++++++++
 5 files changed, 76 insertions(+)

diff --git a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto
index 9511ee4..fc97290 100644
--- a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto
+++ b/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 {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
index dae7b94..8157af9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
+++ b/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;
+  }
+
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
index 06564a9..e944062 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
+++ b/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;
+  }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java
index 3d32ea3..409e912c 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCloneSnapshotProcedure.java
+++ b/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;
@@ -163,6 +164,31 @@ public class TestCloneSnapshotProcedure extends TestTableDDLProcedureBase {
   }
 
   @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();
     final TableName clonedTableName = TableName.valueOf("testRollbackAndDoubleExecution");
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java
index 7e5892b..77c4535 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java
@@ -211,6 +211,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);