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


##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java:
##########
@@ -144,21 +152,21 @@ public void execute() throws IOException {
         // logs while we do the backup.
         backupManager.writeBackupStartCode(0L);
       }
-      // We roll log here before we do the snapshot. It is possible there is 
duplicate data
-      // in the log that is already in the snapshot. But if we do it after the 
snapshot, we
-      // could have data loss.
-      // A better approach is to do the roll log on each RS in the same global 
procedure as
-      // the snapshot.
-      LOG.info("Execute roll log procedure for full backup ...");
 
       // Gather the bulk loads being tracked by the system, which can be 
deleted (since their data
       // will be part of the snapshot being taken). We gather this list before 
taking the actual
       // snapshots for the same reason as the log rolls.
       List<BulkLoad> bulkLoadsToDelete = 
backupManager.readBulkloadRows(tableList);
+      Map<String, Long> previousLogRollsByHost = 
backupManager.readRegionServerLastLogRollResult();
 
+      // We roll log here before we do the snapshot. It is possible there is 
duplicate data

Review Comment:
   Nit: this comment block can be improved a bit.
   
   - Won't there always be data duplication between the rolled log and the 
snapshot? I don't see a reason to mention this.
   - I would mention that we roll the logs to create a boundary for WALs that 
need to be tracked for upcoming incremental backups.
   - `A better approach is` -> `...would be` (to avoid confusion that you're 
talking about what is actually happen



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