DieterDP-ng commented on code in PR #5868:
URL: https://github.com/apache/hbase/pull/5868#discussion_r1589353145


##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupManager.java:
##########
@@ -295,47 +295,10 @@ public ArrayList<BackupImage> getAncestors(BackupInfo 
backupInfo) throws IOExcep
         
.withRootDir(backup.getBackupRootDir()).withTableList(backup.getTableNames())
         
.withStartTime(backup.getStartTs()).withCompleteTime(backup.getCompleteTs()).build();
 
-      // Only direct ancestors for a backup are required and not entire 
history of backup for this
-      // table resulting in verifying all of the previous backups which is 
unnecessary and backup
-      // paths need not be valid beyond the lifetime of a backup.
-      //
-      // RootDir is way of grouping a single backup including one full and 
many incremental backups
-      if (!image.getRootDir().equals(backupInfo.getBackupRootDir())) {
-        continue;
-      }
-
-      // add the full backup image as an ancestor until the last incremental 
backup
+      LOG.debug("Dependent incremental backup image: {BackupID={}}", 
image.getBackupId());
+      ancestors.add(image);
       if (backup.getType().equals(BackupType.FULL)) {
-        // check the backup image coverage, if previous image could be covered 
by the newer ones,
-        // then no need to add
-        if (!BackupManifest.canCoverImage(ancestors, image)) {
-          ancestors.add(image);
-        }
-      } else {
-        // found last incremental backup, if previously added full backup 
ancestor images can cover
-        // it, then this incremental ancestor is not the dependent of the 
current incremental
-        // backup, that is to say, this is the backup scope boundary of 
current table set.
-        // Otherwise, this incremental backup ancestor is the dependent 
ancestor of the ongoing
-        // incremental backup
-        if (BackupManifest.canCoverImage(ancestors, image)) {
-          LOG.debug("Met the backup boundary of the current table set:");
-          for (BackupImage image1 : ancestors) {
-            LOG.debug("  BackupID={}, BackupDir={}", image1.getBackupId(), 
image1.getRootDir());
-          }
-        } else {
-          Path logBackupPath =
-            HBackupFileSystem.getBackupPath(backup.getBackupRootDir(), 
backup.getBackupId());
-          LOG.debug(
-            "Current backup has an incremental backup ancestor, "
-              + "touching its image manifest in {}" + " to construct the 
dependency.",
-            logBackupPath.toString());
-          BackupManifest lastIncrImgManifest = new BackupManifest(conf, 
logBackupPath);
-          BackupImage lastIncrImage = lastIncrImgManifest.getBackupImage();
-          ancestors.add(lastIncrImage);
-
-          LOG.debug("Last dependent incremental backup image: {BackupID={}" + 
"BackupDir={}}",
-            lastIncrImage.getBackupId(), lastIncrImage.getRootDir());
-        }
+        break;

Review Comment:
   I was under the (wrong) impression that incremental backups always built on 
a single full backup.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to