HBASE-29448 Modern backup failures can cause backup system to lock up Co-authored-by: Hernan Gelaf-Romer <hgelafromer@hubspot.com> Signed-off-by: Charles Connell <cconnell@apache.org> Signed-off-by: Ray Mattingly <rmattingly@apache.org>
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java index 61a7445..f2ddcf5 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
@@ -1403,9 +1403,7 @@ try (Admin admin = conn.getAdmin()) { String snapshotName = BackupSystemTable.getSnapshotName(conf); if (snapshotExists(admin, snapshotName)) { - admin.disableTable(BackupSystemTable.getTableName(conf)); - admin.restoreSnapshot(snapshotName); - admin.enableTable(BackupSystemTable.getTableName(conf)); + admin.restoreBackupSystemTable(snapshotName); LOG.debug("Done restoring backup system table"); } else { // Snapshot does not exists, i.e completeBackup failed after
diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestRestoreBackupSystemTable.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestRestoreBackupSystemTable.java new file mode 100644 index 0000000..31ded67 --- /dev/null +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestRestoreBackupSystemTable.java
@@ -0,0 +1,84 @@ +/* + * 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.backup.master; + +import static org.junit.Assert.assertEquals; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.backup.impl.BackupSystemTable; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.testclassification.MasterTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ MasterTests.class, MediumTests.class }) +public class TestRestoreBackupSystemTable { + private static final String BACKUP_ROOT = "root"; + private static final HBaseTestingUtil UTIL = new HBaseTestingUtil(); + + @BeforeClass + public static void setUp() throws Exception { + UTIL.startMiniCluster(); + } + + @Test + public void itRestoresFromSnapshot() throws Exception { + BackupSystemTable table = new BackupSystemTable(UTIL.getConnection()); + Set<TableName> tables = new HashSet<>(); + + tables.add(TableName.valueOf("test1")); + tables.add(TableName.valueOf("test2")); + tables.add(TableName.valueOf("test3")); + + Map<String, Long> rsTimestampMap = new HashMap<>(); + rsTimestampMap.put("rs1:100", 100L); + rsTimestampMap.put("rs2:100", 101L); + rsTimestampMap.put("rs3:100", 103L); + + table.writeRegionServerLogTimestamp(tables, rsTimestampMap, BACKUP_ROOT); + BackupSystemTable.snapshot(UTIL.getConnection()); + + Admin admin = UTIL.getAdmin(); + TableName backupSystemTn = BackupSystemTable.getTableName(UTIL.getConfiguration()); + admin.disableTable(backupSystemTn); + admin.truncateTable(backupSystemTn, true); + + BackupSystemTable.restoreFromSnapshot(UTIL.getConnection()); + Map<TableName, Map<String, Long>> results = table.readLogTimestampMap(BACKUP_ROOT); + + assertEquals(results.size(), tables.size()); + + for (TableName tableName : tables) { + Map<String, Long> resultMap = results.get(tableName); + assertEquals(resultMap, rsTimestampMap); + } + } + + @AfterClass + public static void tearDown() throws Exception { + UTIL.shutdownMiniCluster(); + } +}
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java index 43a004a..1c08ec3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -2661,4 +2661,7 @@ * Get the list of cached files */ List<String> getCachedFilesList(ServerName serverName) throws IOException; + + @InterfaceAudience.Private + void restoreBackupSystemTable(String snapshotName) throws IOException; }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java index c866f43..e6bf6c3 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
@@ -1141,4 +1141,9 @@ public List<String> getCachedFilesList(ServerName serverName) throws IOException { return get(admin.getCachedFilesList(serverName)); } + + @Override + public void restoreBackupSystemTable(String snapshotName) throws IOException { + get(admin.restoreBackupSystemTable(snapshotName)); + } }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java index d808aec..ec0556f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
@@ -1871,4 +1871,7 @@ * Get the list of cached files */ CompletableFuture<List<String>> getCachedFilesList(ServerName serverName); + + @InterfaceAudience.Private + CompletableFuture<Void> restoreBackupSystemTable(String snapshotName); }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java index 33ac47c..b1fb2be 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
@@ -1010,4 +1010,9 @@ public CompletableFuture<List<String>> getCachedFilesList(ServerName serverName) { return wrap(rawAdmin.getCachedFilesList(serverName)); } + + @Override + public CompletableFuture<Void> restoreBackupSystemTable(String snapshotName) { + return wrap(rawAdmin.restoreBackupSystemTable(snapshotName)); + } }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index 2373e93..710c8c4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -2795,6 +2795,19 @@ } } + private static class RestoreBackupSystemTableProcedureBiConsumer extends ProcedureBiConsumer { + + @Override + void onFinished() { + LOG.info("RestoreBackupSystemTableProcedure completed"); + } + + @Override + void onError(Throwable error) { + LOG.info("RestoreBackupSystemTableProcedure failed with {}", error.getMessage()); + } + } + private static class CreateTableProcedureBiConsumer extends TableProcedureBiConsumer { CreateTableProcedureBiConsumer(TableName tableName) { @@ -4637,4 +4650,16 @@ resp -> resp.getCachedFilesList())) .serverName(serverName).call(); } + + @Override + public CompletableFuture<Void> restoreBackupSystemTable(String snapshotName) { + MasterProtos.RestoreBackupSystemTableRequest request = + MasterProtos.RestoreBackupSystemTableRequest.newBuilder().setSnapshotName(snapshotName) + .build(); + return this.<MasterProtos.RestoreBackupSystemTableRequest, + MasterProtos.RestoreBackupSystemTableResponse> procedureCall(request, + MasterService.Interface::restoreBackupSystemTable, + MasterProtos.RestoreBackupSystemTableResponse::getProcId, + new RestoreBackupSystemTableProcedureBiConsumer()); + } }
diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto index 768a1d7..6dd6ee7 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto
@@ -1280,6 +1280,9 @@ rpc FlushTable(FlushTableRequest) returns(FlushTableResponse); + rpc RestoreBackupSystemTable(RestoreBackupSystemTableRequest) + returns(RestoreBackupSystemTableResponse); + rpc rollAllWALWriters(RollAllWALWritersRequest) returns(RollAllWALWritersResponse); } @@ -1369,6 +1372,13 @@ message FixMetaResponse {} +message RestoreBackupSystemTableRequest { + required string snapshot_name = 1; +} +message RestoreBackupSystemTableResponse { + optional uint64 proc_id = 1; +} + service HbckService { /** Update state of the table in meta only*/ rpc SetTableStateInMeta(SetTableStateInMetaRequest)
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 554d7ec..7e6c6c8 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto
@@ -840,6 +840,13 @@ optional ForeignExceptionMessage error = 2; } +enum RestoreBackupSystemTableState { + RESTORE_BACKUP_SYSTEM_TABLE_PREPARE = 1; + RESTORE_BACKUP_SYSTEM_TABLE_DISABLE = 2; + RESTORE_BACKUP_SYSTEM_TABLE_RESTORE = 3; + RESTORE_BACKUP_SYSTEM_TABLE_ENABLE = 4; +} + enum LogRollProcedureState { LOG_ROLL_ROLL_LOG_ON_RS = 1; LOG_ROLL_COLLECT_RS_HIGHEST_WAL_FILENUM = 2;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index de911b5..e9e0f97 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -76,6 +76,7 @@ import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.procedure.MasterProcedureUtil; import org.apache.hadoop.hbase.master.procedure.MasterProcedureUtil.NonceProcedureRunnable; +import org.apache.hadoop.hbase.master.procedure.RestoreBackupSystemTableProcedure; import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure; import org.apache.hadoop.hbase.master.replication.AbstractPeerNoLockProcedure; import org.apache.hadoop.hbase.mob.MobUtils; @@ -3668,6 +3669,24 @@ } @Override + public MasterProtos.RestoreBackupSystemTableResponse restoreBackupSystemTable( + RpcController rpcController, + MasterProtos.RestoreBackupSystemTableRequest restoreBackupSystemTableRequest) + throws ServiceException { + try { + String snapshotName = restoreBackupSystemTableRequest.getSnapshotName(); + SnapshotDescription snapshot = server.snapshotManager.getCompletedSnapshots().stream() + .filter(s -> s.getName().equals(snapshotName)).findFirst() + .orElseThrow(() -> new ServiceException("Snapshot %s not found".formatted(snapshotName))); + long pid = server.getMasterProcedureExecutor() + .submitProcedure(new RestoreBackupSystemTableProcedure(snapshot)); + return MasterProtos.RestoreBackupSystemTableResponse.newBuilder().setProcId(pid).build(); + } catch (IOException e) { + throw new ServiceException(e); + } + } + + @Override public RollAllWALWritersResponse rollAllWALWriters(RpcController rpcController, RollAllWALWritersRequest request) throws ServiceException { try {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreBackupSystemTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreBackupSystemTableProcedure.java new file mode 100644 index 0000000..af980db --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreBackupSystemTableProcedure.java
@@ -0,0 +1,169 @@ +/* + * 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.master.procedure; + +import java.io.IOException; +import java.util.List; +import org.apache.hadoop.hbase.HBaseIOException; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableState; +import org.apache.hadoop.hbase.procedure2.Procedure; +import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException; +import org.apache.hadoop.hbase.procedure2.ProcedureYieldException; +import org.apache.hadoop.hbase.snapshot.SnapshotDoesNotExistException; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; +import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.RestoreBackupSystemTableState; +import org.apache.hadoop.hbase.shaded.protobuf.generated.SnapshotProtos.SnapshotDescription; + +@InterfaceAudience.Private +public class RestoreBackupSystemTableProcedure + extends AbstractStateMachineTableProcedure<RestoreBackupSystemTableState> { + private static final Logger LOG = + LoggerFactory.getLogger(RestoreBackupSystemTableProcedure.class); + + private final SnapshotDescription snapshot; + private boolean enableOnRollback = false; + + // Necessary for the procedure framework. Do not remove. + public RestoreBackupSystemTableProcedure() { + this(null); + } + + public RestoreBackupSystemTableProcedure(SnapshotDescription snapshot) { + this.snapshot = snapshot; + } + + @Override + public TableName getTableName() { + return TableName.valueOf(snapshot.getTable()); + } + + @Override + public TableOperationType getTableOperationType() { + return TableOperationType.RESTORE_BACKUP_SYSTEM_TABLE; + } + + @Override + protected Flow executeFromState(MasterProcedureEnv env, RestoreBackupSystemTableState state) + throws ProcedureSuspendedException, ProcedureYieldException, InterruptedException { + LOG.info("{} execute state={}", this, state); + + try { + switch (state) { + case RESTORE_BACKUP_SYSTEM_TABLE_PREPARE: + prepare(env); + return moreState(RestoreBackupSystemTableState.RESTORE_BACKUP_SYSTEM_TABLE_DISABLE); + case RESTORE_BACKUP_SYSTEM_TABLE_DISABLE: + TableState tableState = + env.getMasterServices().getTableStateManager().getTableState(getTableName()); + if (tableState.isEnabled()) { + addChildProcedure(createDisableTableProcedure(env)); + } + return moreState(RestoreBackupSystemTableState.RESTORE_BACKUP_SYSTEM_TABLE_RESTORE); + case RESTORE_BACKUP_SYSTEM_TABLE_RESTORE: + addChildProcedure(createRestoreSnapshotProcedure(env)); + return moreState(RestoreBackupSystemTableState.RESTORE_BACKUP_SYSTEM_TABLE_ENABLE); + case RESTORE_BACKUP_SYSTEM_TABLE_ENABLE: + addChildProcedure(createEnableTableProcedure(env)); + return Flow.NO_MORE_STATE; + default: + throw new UnsupportedOperationException("unhandled state=" + state); + } + } catch (Exception e) { + setFailure("restore-backup-system-table", e); + LOG.warn("unexpected exception while execute {}. Mark procedure Failed.", this, e); + return Flow.NO_MORE_STATE; + } + } + + @Override + protected void rollbackState(MasterProcedureEnv env, RestoreBackupSystemTableState state) + throws IOException, InterruptedException { + switch (state) { + case RESTORE_BACKUP_SYSTEM_TABLE_DISABLE, RESTORE_BACKUP_SYSTEM_TABLE_PREPARE: + return; + case RESTORE_BACKUP_SYSTEM_TABLE_RESTORE, RESTORE_BACKUP_SYSTEM_TABLE_ENABLE: + if (enableOnRollback) { + addChildProcedure(createEnableTableProcedure(env)); + } + return; + default: + throw new UnsupportedOperationException("unhandled state=" + state); + } + } + + @Override + protected RestoreBackupSystemTableState getState(int stateId) { + return RestoreBackupSystemTableState.forNumber(stateId); + } + + @Override + protected int getStateId(RestoreBackupSystemTableState state) { + return state.getNumber(); + } + + @Override + protected RestoreBackupSystemTableState getInitialState() { + return RestoreBackupSystemTableState.RESTORE_BACKUP_SYSTEM_TABLE_PREPARE; + } + + private Flow moreState(RestoreBackupSystemTableState next) { + setNextState(next); + return Flow.HAS_MORE_STATE; + } + + private Procedure<MasterProcedureEnv>[] createDisableTableProcedure(MasterProcedureEnv env) + throws HBaseIOException { + DisableTableProcedure disableTableProcedure = + new DisableTableProcedure(env, getTableName(), true); + return new DisableTableProcedure[] { disableTableProcedure }; + } + + private Procedure<MasterProcedureEnv>[] createEnableTableProcedure(MasterProcedureEnv env) { + EnableTableProcedure enableTableProcedure = new EnableTableProcedure(env, getTableName()); + return new EnableTableProcedure[] { enableTableProcedure }; + } + + private Procedure<MasterProcedureEnv>[] createRestoreSnapshotProcedure(MasterProcedureEnv env) + throws IOException { + TableDescriptor desc = env.getMasterServices().getTableDescriptors().get(getTableName()); + RestoreSnapshotProcedure restoreSnapshotProcedure = + new RestoreSnapshotProcedure(env, desc, snapshot); + return new RestoreSnapshotProcedure[] { restoreSnapshotProcedure }; + } + + private void prepare(MasterProcedureEnv env) throws IOException { + List<SnapshotDescription> snapshots = + env.getMasterServices().getSnapshotManager().getCompletedSnapshots(); + boolean exists = snapshots.stream().anyMatch(s -> s.getName().equals(snapshot.getName())); + if (!exists) { + throw new SnapshotDoesNotExistException(ProtobufUtil.createSnapshotDesc(snapshot)); + } + + TableState tableState = + env.getMasterServices().getTableStateManager().getTableState(getTableName()); + if (tableState.isEnabled()) { + enableOnRollback = true; + } + } +}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableProcedureInterface.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableProcedureInterface.java index 00b9776..c5c7ec6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableProcedureInterface.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableProcedureInterface.java
@@ -50,7 +50,8 @@ REGION_UNASSIGN, REGION_GC, MERGED_REGIONS_GC/* region operations */, - REGION_TRUNCATE + REGION_TRUNCATE, + RESTORE_BACKUP_SYSTEM_TABLE } /** Returns the name of the table the procedure is operating on */
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java index be66a28..7be4c4b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TableQueue.java
@@ -54,6 +54,7 @@ case DISABLE: case SNAPSHOT: case ENABLE: + case RESTORE_BACKUP_SYSTEM_TABLE: return true; case EDIT: // we allow concurrent edit on the ns family in meta table
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java index 4d592b4..a59b296 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java
@@ -985,6 +985,11 @@ } @Override + public void restoreBackupSystemTable(String snapshotName) throws IOException { + admin.restoreBackupSystemTable(snapshotName); + } + + @Override public boolean replicationPeerModificationSwitch(boolean on, boolean drainProcedures) throws IOException { return admin.replicationPeerModificationSwitch(on, drainProcedures);
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java index a0d73dc..3d5a7e5 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java
@@ -1360,6 +1360,11 @@ } @Override + public void restoreBackupSystemTable(String snapshotName) throws IOException { + throw new NotImplementedException("restoreBackupSystemTable not supported in ThriftAdmin"); + } + + @Override public boolean replicationPeerModificationSwitch(boolean on, boolean drainProcedures) throws IOException { throw new NotImplementedException(