HBASE-28539 Fix merging of incremental backups when the backup filesy…

…stem is not the same as the one underpinning HBase itself.
apache · May 6, 2024 · 0b6738e · 0b6738e
1 parent 3d66866
commit 0b6738e
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 3 deletions.
diff --git a/...ackup/src/main/java/org/apache/hadoop/hbase/backup/mapreduce/MapReduceBackupMergeJob.java b/...ackup/src/main/java/org/apache/hadoop/hbase/backup/mapreduce/MapReduceBackupMergeJob.java
@@ -96,7 +96,7 @@ public void run(String[] backupIds) throws IOException {
     boolean finishedTables = false;
     Connection conn = ConnectionFactory.createConnection(getConf());
     BackupSystemTable table = new BackupSystemTable(conn);
-    FileSystem fs = FileSystem.get(getConf());
+    FileSystem fs = null;
 
     try {
 
@@ -112,6 +112,8 @@ public void run(String[] backupIds) throws IOException {
 
       BackupInfo bInfo = table.readBackupInfo(backupIds[0]);
       String backupRoot = bInfo.getBackupRootDir();
+      Path backupRootPath = new Path(backupRoot);
+      fs = backupRootPath.getFileSystem(conf);
 
       for (int i = 0; i < tableNames.length; i++) {
         LOG.info("Merge backup images for " + tableNames[i]);
@@ -120,7 +122,9 @@ public void run(String[] backupIds) throws IOException {
         Path[] dirPaths = findInputDirectories(fs, backupRoot, tableNames[i], backupIds);
         String dirs = StringUtils.join(dirPaths, ",");
 
-        Path bulkOutputPath = BackupUtils.getBulkOutputDir(
+        // bulkOutputPath should be on the same filesystem as backupRoot
+        Path tmpRestoreOutputDir = HBackupFileSystem.getBackupTmpDirPath(backupRoot);
+        Path bulkOutputPath = BackupUtils.getBulkOutputDir(tmpRestoreOutputDir,
           BackupUtils.getFileNameCompatibleString(tableNames[i]), getConf(), false);
         // Delete content if exists
         if (fs.exists(bulkOutputPath)) {
@@ -186,7 +190,9 @@ public void run(String[] backupIds) throws IOException {
       if (!finishedTables) {
         // cleanup bulk directories and finish merge
         // merge MUST be repeated (no need for repair)
-        cleanupBulkLoadDirs(fs, toPathList(processedTableList));
+        if (fs != null) {
+          cleanupBulkLoadDirs(fs, toPathList(processedTableList));
+        }
         table.finishMergeOperation();
         table.finishBackupExclusiveOperation();
         throw new IOException("Backup merge operation failed, you should try it again", e);

diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupMerge.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupMerge.java
@@ -17,9 +17,13 @@
  */
 package org.apache.hadoop.hbase.backup;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.io.File;
 import java.util.List;
+import java.util.UUID;
+import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.backup.impl.BackupAdminImpl;
@@ -124,4 +128,44 @@ public void TestIncBackupMergeRestore() throws Exception {
     admin.close();
     conn.close();
   }
+
+  @Test
+  public void TestIncBackupMergeRestoreSeparateFs() throws Exception {
+
+    // prepare BACKUP_ROOT_DIR on a different filesystem from HBase
+    File tempDir = new File(FileUtils.getTempDirectory(), UUID.randomUUID().toString());
+    tempDir.deleteOnExit();
+    BACKUP_ROOT_DIR = tempDir.toURI().toString();
+    System.out.println(BACKUP_ROOT_DIR);
+
+    Connection conn = ConnectionFactory.createConnection(conf1);
+    BackupAdminImpl client = new BackupAdminImpl(conn);
+    List<TableName> tables = Lists.newArrayList(table1, table2);
+
+    BackupRequest request = createBackupRequest(BackupType.FULL, tables, BACKUP_ROOT_DIR);
+    String backupIdFull = client.backupTables(request);
+    assertTrue(checkSucceeded(backupIdFull));
+
+    request = createBackupRequest(BackupType.INCREMENTAL, tables, BACKUP_ROOT_DIR);
+    String backupIdIncMultiple = client.backupTables(request);
+    assertTrue(checkSucceeded(backupIdIncMultiple));
+
+    request = createBackupRequest(BackupType.INCREMENTAL, tables, BACKUP_ROOT_DIR);
+    String backupIdIncMultiple2 = client.backupTables(request);
+    assertTrue(checkSucceeded(backupIdIncMultiple2));
+
+    try (BackupAdmin bAdmin = new BackupAdminImpl(conn)) {
+      String[] backups = new String[] { backupIdIncMultiple, backupIdIncMultiple2 };
+      // this throws java.lang.IllegalArgumentException: Wrong FS prior to HBASE-28539
+      bAdmin.mergeBackups(backups);
+    }
+    assertTrue(
+      new File(HBackupFileSystem.getBackupPath(BACKUP_ROOT_DIR, backupIdFull).toUri()).exists());
+    assertFalse(
+      new File(HBackupFileSystem.getBackupPath(BACKUP_ROOT_DIR, backupIdIncMultiple).toUri())
+        .exists());
+    assertTrue(
+      new File(HBackupFileSystem.getBackupPath(BACKUP_ROOT_DIR, backupIdIncMultiple2).toUri())
+        .exists());
+  }
 }