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


##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupManager.java:
##########
@@ -295,51 +298,31 @@ 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 the image has a different rootDir, it cannot be an ancestor.
       if (!image.getRootDir().equals(backupInfo.getBackupRootDir())) {
         continue;
       }
 
-      // add the full backup image as an ancestor until the last incremental 
backup
-      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);
-        }
+      // The ancestors consist of the most recent FULL backups that cover the 
list of tables
+      // required in the new backup and all INCREMENTAL backups that came 
after one of those FULL
+      // backups.
+      if (backup.getType().equals(BackupType.INCREMENTAL)) {

Review Comment:
   I did the following experiment:
   
   ```
   echo "create 'table1', 'cf'" | bin/hbase shell -n
   echo "create 'table2', 'cf'" | bin/hbase shell -n
   echo "create 'table3', 'cf'" | bin/hbase shell -n
   bin/hbase backup create full file:/tmp/hbasebackups -t table1,table2
   echo "put 'table1', 'row1', 'cf:a', 'valueA'" | bin/hbase shell -n
   echo "put 'table2', 'row1', 'cf:a', 'valueB'" | bin/hbase shell -n
   bin/hbase backup create incremental file:/tmp/hbasebackups -t table1
   bin/hbase backup history
   
   
{ID=backup_1714763143839,Type=INCREMENTAL,Tables={table2,table1},State=COMPLETE,Start
 time=Fri May 03 21:05:44 CEST 2024,End time=Fri May 03 21:05:47 CEST 
2024,Progress=100%}
   
{ID=backup_1714763125413,Type=FULL,Tables={table2,table1},State=COMPLETE,Start 
time=Fri May 03 21:05:25 CEST 2024,End time=Fri May 03 21:05:29 CEST 
2024,Progress=100%}
   ```
   
   This seems to suggest that the incremental backup includes both tables, even 
though I only requested table1. So I don't think it's possible to create an 
incremental backup of just a single table if your FULL backups have more tables 
covered?



-- 
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