HBASE-28687 BackupSystemTable#checkSystemTable should ensure system t…

…ables are enabled (apache#6018) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
ndimiduk · Jun 26, 2024 · 9c713fb · 9c713fb
1 parent d066045
commit 9c713fb
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 1 deletion.
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
@@ -47,6 +47,7 @@
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableExistsException;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotDisabledException;
 import org.apache.hadoop.hbase.backup.BackupInfo;
 import org.apache.hadoop.hbase.backup.BackupInfo.BackupState;
 import org.apache.hadoop.hbase.backup.BackupRestoreConstants;
@@ -207,10 +208,12 @@ private void checkSystemTable() throws IOException {
         TableDescriptor backupHTD = BackupSystemTable.getSystemTableDescriptor(conf);
         createSystemTable(admin, backupHTD);
       }
+      ensureTableEnabled(admin, tableName);
       if (!admin.tableExists(bulkLoadTableName)) {
         TableDescriptor blHTD = BackupSystemTable.getSystemTableForBulkLoadedDataDescriptor(conf);
         createSystemTable(admin, blHTD);
       }
+      ensureTableEnabled(admin, bulkLoadTableName);
       waitForSystemTable(admin, tableName);
       waitForSystemTable(admin, bulkLoadTableName);
     }
@@ -1889,4 +1892,14 @@ private static byte[] rowkey(String s, String... other) {
     }
     return Bytes.toBytes(sb.toString());
   }
+
+  private static void ensureTableEnabled(Admin admin, TableName tableName) throws IOException {
+    if (!admin.isTableEnabled(tableName)) {
+      try {
+        admin.enableTable(tableName);
+      } catch (TableNotDisabledException ignored) {
+        LOG.info("Table {} is not disabled, ignoring enable request", tableName);
+      }
+    }
+  }
 }
diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java
@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -57,6 +58,7 @@
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.master.cleaner.LogCleaner;
 import org.apache.hadoop.hbase.master.cleaner.TimeToLiveLogCleaner;
+import org.apache.hadoop.hbase.regionserver.LogRoller;
 import org.apache.hadoop.hbase.security.HadoopSecurityEnabledUserProviderForTesting;
 import org.apache.hadoop.hbase.security.UserProvider;
 import org.apache.hadoop.hbase.security.access.SecureTestUtil;
@@ -67,6 +69,7 @@
 import org.apache.hadoop.hbase.wal.AbstractFSWALProvider;
 import org.apache.hadoop.hbase.wal.WALFactory;
 import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -115,6 +118,38 @@ public IncrementalTableBackupClientForTest(Connection conn, String backupId,
       super(conn, backupId, request);
     }
 
+    @Before
+    public void ensurePreviousBackupTestsAreCleanedUp() throws Exception {
+      // Every operation here may not be necessary for any given test,
+      // some often being no-ops. the goal is to help ensure atomicity
+      // of that tests that implement TestBackupBase
+      try (BackupAdmin backupAdmin = getBackupAdmin()) {
+        backupManager.finishBackupSession();
+        backupAdmin.listBackupSets().forEach(backupSet -> {
+          try {
+            backupAdmin.deleteBackupSet(backupSet.getName());
+          } catch (IOException ignored) {
+          }
+        });
+      } catch (Exception ignored) {
+      }
+      Arrays.stream(TEST_UTIL.getAdmin().listTableNames())
+        .filter(tableName -> !tableName.isSystemTable()).forEach(tableName -> {
+          try {
+            TEST_UTIL.truncateTable(tableName);
+          } catch (IOException ignored) {
+          }
+        });
+      TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().forEach(rst -> {
+        try {
+          LogRoller walRoller = rst.getRegionServer().getWalRoller();
+          walRoller.requestRollAll();
+          walRoller.waitUntilWalRollFinished();
+        } catch (Exception ignored) {
+        }
+      });
+    }
+
     @Override
     public void execute() throws IOException {
       // case INCREMENTAL_COPY:
@@ -468,7 +503,7 @@ private BackupInfo getBackupInfo(String backupId) throws IOException {
     }
   }
 
-  protected BackupAdmin getBackupAdmin() throws IOException {
+  protected static BackupAdmin getBackupAdmin() throws IOException {
     return new BackupAdminImpl(TEST_UTIL.getConnection());
   }