HBase Backport: Modern backup failures can cause backup system to lock up

Author: rmdmattingly

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java

@@ -1403,9 +1403,7 @@ public static void restoreFromSnapshot(Connection conn) throws IOException {
     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

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.HBaseTestingUtility;
+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 HBaseTestingUtility UTIL = new HBaseTestingUtility();
+
+  @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();
+  }
+}

hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java

@@ -18,7 +18,6 @@
 package org.apache.hadoop.hbase.client;
 
 import static org.apache.hadoop.hbase.util.FutureUtils.get;
-
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.Collection;
@@ -68,7 +67,6 @@
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.yetus.audience.InterfaceAudience;
-
 import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableList;
 
 /**
@@ -3356,4 +3354,15 @@ List<LogEntry> getLogEntries(Set<ServerName> serverNames, String logType, Server
    * Get the list of cached files
    */
   List<String> getCachedFilesList(ServerName serverName) throws IOException;
+
+  @InterfaceAudience.Private
+  default void restoreBackupSystemTable(String snapshotName) throws IOException {
+    SnapshotDescription snapshot = listSnapshots().stream()
+      .filter(s -> s.getName().equals(snapshotName)).findFirst().orElseThrow(() -> new
+        IOException("Snapshot " + snapshotName + " not found"));
+    TableName tn = snapshot.getTableName();
+    disableTable(tn);
+    restoreSnapshot(snapshotName);
+    enableTable(tn);
+  }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java

@@ -1721,4 +1721,7 @@ CompletableFuture<List<LogEntry>> getLogEntries(Set<ServerName> serverNames, Str
    * Get the list of cached files
    */
   CompletableFuture<List<String>> getCachedFilesList(ServerName serverName);
+
+  @InterfaceAudience.Private
+  CompletableFuture<Void> restoreBackupSystemTable(String snapshotName);
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java

@@ -921,4 +921,9 @@ public CompletableFuture<Void> flushMasterStore() {
   public CompletableFuture<List<String>> getCachedFilesList(ServerName serverName) {
     return wrap(rawAdmin.getCachedFilesList(serverName));
   }
+
+  @Override
+  public CompletableFuture<Void> restoreBackupSystemTable(String snapshotName) {
+    return wrap(rawAdmin.restoreBackupSystemTable(snapshotName));
+  }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java

@@ -2082,6 +2082,14 @@ public FlushMasterStoreResponse flushMasterStore(RpcController controller,
         return stub.flushMasterStore(controller, request);
       }
 
+      @Override
+      public MasterProtos.RestoreBackupSystemTableResponse restoreBackupSystemTable(
+        RpcController rpcController,
+        MasterProtos.RestoreBackupSystemTableRequest restoreBackupSystemTableRequest)
+        throws ServiceException {
+        return stub.restoreBackupSystemTable(rpcController, restoreBackupSystemTableRequest);
+      }
+
       @Override
       public ReplicationPeerModificationSwitchResponse replicationPeerModificationSwitch(
         RpcController controller, ReplicationPeerModificationSwitchRequest request)

hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java

@@ -2095,6 +2095,22 @@ public List<String> getCachedFilesList(ServerName serverName) throws IOException
       this.connection.getAdmin(serverName));
   }
 
+  @Override
+  public void restoreBackupSystemTable(String snapshotName) throws IOException {
+    long pid =
+      executeCallable(new MasterCallable<Long>(getConnection(), getRpcControllerFactory()) {
+        @Override
+        protected Long rpcCall() throws Exception {
+          return master.restoreBackupSystemTable(getRpcController(),
+            MasterProtos.RestoreBackupSystemTableRequest.newBuilder().setSnapshotName(snapshotName)
+              .build())
+            .getProcId();
+        }
+      });
+    ProcedureFuture<Void> future = new ProcedureFuture<>(this, pid);
+    get(future, getProcedureTimeout, TimeUnit.MILLISECONDS);
+  }
+
   private MasterCallable<MasterProtos.TruncateRegionResponse>
     getTruncateRegionCallable(TableName tableName, RegionInfo hri) {
     return new MasterCallable<MasterProtos.TruncateRegionResponse>(getConnection(),

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java

@@ -2716,6 +2716,19 @@ void onError(Throwable error) {
     }
   }
 
+  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) {
@@ -4299,4 +4312,16 @@ List<String>> adminCall(controller, stub, request.build(),
           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());
+  }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/client/ShortCircuitMasterConnection.java

@@ -796,6 +796,13 @@ public FlushMasterStoreResponse flushMasterStore(RpcController controller,
     return stub.flushMasterStore(controller, request);
   }
 
+  @Override
+  public MasterProtos.RestoreBackupSystemTableResponse restoreBackupSystemTable(
+    RpcController controller, MasterProtos.RestoreBackupSystemTableRequest request)
+    throws ServiceException {
+    return stub.restoreBackupSystemTable(controller, request);
+  }
+
   @Override
   public ReplicationPeerModificationSwitchResponse replicationPeerModificationSwitch(
     RpcController controller, ReplicationPeerModificationSwitchRequest request)

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

@@ -1228,6 +1228,9 @@ service MasterService {
 
   rpc FlushMasterStore(FlushMasterStoreRequest)
     returns(FlushMasterStoreResponse);
+
+  rpc RestoreBackupSystemTable(RestoreBackupSystemTableRequest)
+    returns(RestoreBackupSystemTableResponse);
 }
 
 // HBCK Service definitions.
@@ -1313,6 +1316,13 @@ message FixMetaRequest {}
 
 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)