HBASE-28882 Backup restores are broken if the backup has moved locations

Author: Ray Mattingly

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/HBackupFileSystem.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.backup;
 
+import com.google.errorprone.annotations.RestrictedApi;
 import java.io.IOException;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
@@ -27,6 +28,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
+
 /**
  * View to an on-disk Backup Image FileSytem Provides the set of methods necessary to interact with
  * the on-disk Backup Image data.
@@ -97,10 +100,18 @@ public static Path getTableBackupPath(TableName tableName, Path backupRootPath,
 
   private static Path getManifestPath(Configuration conf, Path backupRootPath, String backupId)
     throws IOException {
+    return getManifestPath(conf, backupRootPath, backupId, true);
+  }
+
+  /* Visible for testing only */
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+      allowedOnPath = ".*/src/test/.*")
+  protected static Path getManifestPath(Configuration conf, Path backupRootPath, String backupId,
+    boolean throwIfNotFound) throws IOException {
     FileSystem fs = backupRootPath.getFileSystem(conf);
     Path manifestPath = new Path(getBackupPath(backupRootPath.toString(), backupId) + Path.SEPARATOR
       + BackupManifest.MANIFEST_FILE_NAME);
-    if (!fs.exists(manifestPath)) {
+    if (throwIfNotFound && !fs.exists(manifestPath)) {
       String errorMsg = "Could not find backup manifest " + BackupManifest.MANIFEST_FILE_NAME
         + " for " + backupId + ". File " + manifestPath + " does not exists. Did " + backupId
         + " correspond to previously taken backup ?";
@@ -109,6 +120,16 @@ private static Path getManifestPath(Configuration conf, Path backupRootPath, Str
     return manifestPath;
   }
 
+  /**
+   * The inverse of {@link #getManifestPath(Configuration, Path, String)}
+   */
+  public static Path getRootDirFromManifestPath(Path manifestPath, String backupId) {
+    Preconditions.checkArgument(manifestPath.getParent().toString().endsWith(backupId),
+      String.format("Manifest path parent %s must end in backupId %s", manifestPath, backupId));
+    return new Path(
+      manifestPath.toString().substring(0, manifestPath.toString().indexOf("/" + backupId)));
+  }
+
   public static BackupManifest getManifest(Configuration conf, Path backupRootPath, String backupId)
     throws IOException {
     BackupManifest manifest =

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

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.backup.impl;
 
+import com.google.errorprone.annotations.RestrictedApi;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -439,7 +440,7 @@ public BackupManifest(FileSystem fs, Path backupPath) throws BackupException {
           } catch (Exception e) {
             throw new BackupException(e);
           }
-          this.backupImage = BackupImage.fromProto(proto);
+          this.backupImage = hydrateRootDir(BackupImage.fromProto(proto), backupPath);
           LOG.debug("Loaded manifest instance from manifest file: "
             + BackupUtils.getPath(subFile.getPath()));
           return;
@@ -452,6 +453,25 @@ public BackupManifest(FileSystem fs, Path backupPath) throws BackupException {
     }
   }
 
+  /* Visible for testing only */
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+      allowedOnPath = ".*/src/test/.*")
+  public static BackupImage hydrateRootDir(BackupImage backupImage, Path backupPath)
+    throws IOException {
+    String providedRootDir =
+      HBackupFileSystem.getRootDirFromManifestPath(backupPath, backupImage.backupId).toString();
+    if (!providedRootDir.startsWith(backupImage.getRootDir())) {
+      LOG.info(
+        "Provided backup root ({}) is different from the BackupImage's root ({}). We will prefer the provided root for this Backup and its ancestors.",
+        providedRootDir, backupImage.getRootDir());
+      backupImage.setRootDir(providedRootDir);
+      for (BackupImage ancestor : backupImage.getAncestors()) {
+        ancestor.setRootDir(providedRootDir);
+      }
+    }
+    return backupImage;
+  }
+
   public BackupType getType() {
     return backupImage.getType();
   }

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestHBackupFileSystem.java

@@ -0,0 +1,48 @@
+/*
+ * 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;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(SmallTests.class)
+public class TestHBackupFileSystem {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestHBackupFileSystem.class);
+
+  private static final Path ROOT_DIR = new Path("/backup/root");
+  private static final String BACKUP_ID = "123";
+
+  @Test
+  public void testRootDirManifestPathConversion() throws IOException {
+    Path manifestPath =
+      HBackupFileSystem.getManifestPath(new Configuration(), ROOT_DIR, BACKUP_ID, false);
+    Path convertedRootDir = HBackupFileSystem.getRootDirFromManifestPath(manifestPath, BACKUP_ID);
+    assertEquals(ROOT_DIR, convertedRootDir);
+  }
+}

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackup.java

@@ -20,6 +20,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
@@ -104,6 +105,7 @@ public void TestIncBackupRestore() throws Exception {
       BackupAdminImpl client = new BackupAdminImpl(conn);
       BackupRequest request = createBackupRequest(BackupType.FULL, tables, BACKUP_ROOT_DIR);
       String backupIdFull = client.backupTables(request);
+      validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdFull);
       assertTrue(checkSucceeded(backupIdFull));
 
       // #2 - insert some data to table
@@ -156,6 +158,7 @@ public void TestIncBackupRestore() throws Exception {
       BackupManifest manifest =
         HBackupFileSystem.getManifest(conf1, new Path(BACKUP_ROOT_DIR), backupIdIncMultiple);
       assertEquals(Sets.newHashSet(table1, table2), new HashSet<>(manifest.getTableList()));
+      validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdIncMultiple);
 
       // add column family f2 to table1
       // drop column family f3
@@ -176,6 +179,7 @@ public void TestIncBackupRestore() throws Exception {
       request = createBackupRequest(BackupType.INCREMENTAL, tables, BACKUP_ROOT_DIR);
       String backupIdIncMultiple2 = client.backupTables(request);
       assertTrue(checkSucceeded(backupIdIncMultiple2));
+      validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdIncMultiple2);
 
       // #5 - restore full backup for all tables
       TableName[] tablesRestoreFull = new TableName[] { table1, table2 };
@@ -227,4 +231,21 @@ public void TestIncBackupRestore() throws Exception {
       admin.close();
     }
   }
+
+  /**
+   * Check that backup manifest can be produced for a different root. Users may want to move
+   * existing backups to a different location.
+   */
+  private void validateRootPathCanBeOverridden(String originalPath, String backupId)
+    throws IOException {
+    String anotherRootDir = "/some/other/root/dir";
+    Path anotherPath = new Path(anotherRootDir, backupId);
+    BackupManifest.BackupImage differentLocationImage = BackupManifest.hydrateRootDir(
+      HBackupFileSystem.getManifest(conf1, new Path(originalPath), backupId).getBackupImage(),
+      anotherPath);
+    assertEquals(differentLocationImage.getRootDir(), anotherRootDir);
+    for (BackupManifest.BackupImage ancestor : differentLocationImage.getAncestors()) {
+      assertEquals(ancestor.getRootDir(), anotherRootDir);
+    }
+  }
 }