diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto index 01121699880..6746e093885 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/master/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 { @@ -654,4 +656,4 @@ enum ModifyTableDescriptorState { message ModifyTableDescriptorStateData { required TableSchema unmodified_table_schema = 1; optional TableSchema modified_table_schema = 2; -} \ No newline at end of file +} 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 dae7b94134f..8157af99ba4 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 fe3da9aaf7c..a5a18b06112 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; @@ -272,6 +273,7 @@ public class RestoreSnapshotProcedure restoreSnapshotMsg.addParentToChildRegionsPairList (parentToChildrenPair); } } + restoreSnapshotMsg.setRestoreAcl(restoreAcl); serializer.serialize(restoreSnapshotMsg.build()); } @@ -321,6 +323,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 63a50c5d33e..6dd606dcd5f 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; @@ -162,6 +163,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 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 procExec = getMasterProcedureExecutor(); 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 133747933ff..e753ca6da6d 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 @@ -220,6 +220,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 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);