From ff6279862ce74553b1b4f6d4c112dac31d8b94f7 Mon Sep 17 00:00:00 2001 From: heliangjun Date: Sun, 5 Dec 2021 23:25:59 +0800 Subject: [PATCH 1/2] HBASE-26462 Should persist restoreAcl flag in the procedure state for CloneSnapshotProcedure and RestoreSnapshotProcedure --- .../server/master/MasterProcedure.proto | 4 ++- .../procedure/CloneSnapshotProcedure.java | 12 +++++++++ .../procedure/RestoreSnapshotProcedure.java | 11 ++++++++ .../procedure/TestCloneSnapshotProcedure.java | 26 +++++++++++++++++++ .../TestRestoreSnapshotProcedure.java | 19 ++++++++++++++ 5 files changed, 71 insertions(+), 1 deletion(-) 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 01121699880f..6746e0938859 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 dae7b94134fe..3d2b307ea48a 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 @@ -271,6 +271,8 @@ protected void serializeStateData(ProcedureStateSerializer serializer) .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 +305,9 @@ protected void deserializeStateData(ProcedureStateSerializer serializer) 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 +526,11 @@ private void addRegionsToMeta(final MasterProcedureEnv env) throws IOException { metaChanges.updateMetaParentRegions(env.getMasterServices().getConnection(), newRegions); } + /** + * Exposed for Testing: HBASE-26462 + */ + 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 fe3da9aaf7c9..3bad410e7f65 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 @@ -272,6 +272,7 @@ protected void serializeStateData(ProcedureStateSerializer serializer) restoreSnapshotMsg.addParentToChildRegionsPairList (parentToChildrenPair); } } + restoreSnapshotMsg.setRestoreAcl(restoreAcl); serializer.serialize(restoreSnapshotMsg.build()); } @@ -321,6 +322,9 @@ protected void deserializeStateData(ProcedureStateSerializer serializer) parentToChildrenPair.getChild2RegionName())); } } + if (restoreSnapshotMsg.hasRestoreAcl()) { + restoreAcl = restoreSnapshotMsg.getRestoreAcl(); + } } /** @@ -545,4 +549,11 @@ private void restoreSnapshotAcl(final MasterProcedureEnv env) throws IOException env.getMasterServices().getConfiguration()); } } + + /** + * Exposed for Testing: HBASE-26462 + */ + 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 63a50c5d33e8..6dd606dcd5f2 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 void testRecoveryAndDoubleExecution() throws Exception { 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 133747933ff1..5676d62b8437 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 void testRecoveryAndDoubleExecution() throws Exception { 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); From 74930df236eb77264f4ba65f17469dd5db69860f Mon Sep 17 00:00:00 2001 From: liangjunhe <2009bjhlj@gmail.com> Date: Mon, 6 Dec 2021 12:45:49 +0800 Subject: [PATCH 2/2] fix whitespace error and add RestrictedApi annotation --- .../hadoop/hbase/master/procedure/CloneSnapshotProcedure.java | 3 +++ .../hbase/master/procedure/RestoreSnapshotProcedure.java | 3 +++ .../hbase/master/procedure/TestRestoreSnapshotProcedure.java | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) 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 3d2b307ea48a..8157af99ba4b 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; @@ -529,6 +530,8 @@ private void addRegionsToMeta(final MasterProcedureEnv env) throws IOException { /** * 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 3bad410e7f65..a5a18b061125 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; @@ -553,6 +554,8 @@ private void restoreSnapshotAcl(final MasterProcedureEnv env) throws IOException /** * 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/TestRestoreSnapshotProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestRestoreSnapshotProcedure.java index 5676d62b8437..e753ca6da6d6 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 @@ -225,7 +225,7 @@ 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);