From f920d123aca19ba4312c53a71bd485077a530585 Mon Sep 17 00:00:00 2001 From: Ray Mattingly Date: Fri, 21 Jun 2024 12:49:14 -0400 Subject: [PATCH 1/2] HBASE-28687 BackupSystemTable#checkSystemTable should ensure system tables are enabled --- .../apache/hadoop/hbase/backup/impl/BackupSystemTable.java | 4 ++++ 1 file changed, 4 insertions(+) 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 c364316d54eb..1e1d8b5b2e6d 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 @@ -206,10 +206,14 @@ private void checkSystemTable() throws IOException { if (!admin.tableExists(tableName)) { TableDescriptor backupHTD = BackupSystemTable.getSystemTableDescriptor(conf); createSystemTable(admin, backupHTD); + } else if (!admin.isTableEnabled(tableName)) { + admin.enableTable(tableName); } if (!admin.tableExists(bulkLoadTableName)) { TableDescriptor blHTD = BackupSystemTable.getSystemTableForBulkLoadedDataDescriptor(conf); createSystemTable(admin, blHTD); + } else if (!admin.isTableEnabled(bulkLoadTableName)) { + admin.enableTable(bulkLoadTableName); } waitForSystemTable(admin, tableName); waitForSystemTable(admin, bulkLoadTableName); From c684a590407c39ec07183481d9cfb63093fcbc26 Mon Sep 17 00:00:00 2001 From: Ray Mattingly Date: Sun, 23 Jun 2024 16:41:50 -0400 Subject: [PATCH 2/2] catch TableNotDisabledException, less flaky tests --- .../hbase/backup/impl/BackupSystemTable.java | 17 +++++++-- .../hadoop/hbase/backup/TestBackupBase.java | 37 ++++++++++++++++++- 2 files changed, 49 insertions(+), 5 deletions(-) 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 1e1d8b5b2e6d..5a12b45a5861 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 @@ -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; @@ -206,15 +207,13 @@ private void checkSystemTable() throws IOException { if (!admin.tableExists(tableName)) { TableDescriptor backupHTD = BackupSystemTable.getSystemTableDescriptor(conf); createSystemTable(admin, backupHTD); - } else if (!admin.isTableEnabled(tableName)) { - admin.enableTable(tableName); } + ensureTableEnabled(admin, tableName); if (!admin.tableExists(bulkLoadTableName)) { TableDescriptor blHTD = BackupSystemTable.getSystemTableForBulkLoadedDataDescriptor(conf); createSystemTable(admin, blHTD); - } else if (!admin.isTableEnabled(bulkLoadTableName)) { - admin.enableTable(bulkLoadTableName); } + ensureTableEnabled(admin, bulkLoadTableName); waitForSystemTable(admin, tableName); waitForSystemTable(admin, bulkLoadTableName); } @@ -1893,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 index e9c1cfd9c323..ed17ef8a1173 100644 --- 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()); }