DieterDP-ng commented on code in PR #7582:
URL: https://github.com/apache/hbase/pull/7582#discussion_r2746804991
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java:
##########
@@ -228,15 +263,6 @@ private List<String> getLogFilesForNewBackup(Map<String,
Long> olderTimestamps,
} else if (currentLogTS > oldTimeStamp) {
resultLogFiles.add(currentLogFile);
}
-
- // It is possible that a host in .oldlogs is an obsolete region server
Review Comment:
Ah, I assumed you wanted to remove this check because you thought it has no
effect. Now I understand that you're suggesting to change the existing behavior
to include all WALs available at that point in time. (Correct me if I'm wrong
here.)
I think that in most general cases, it's fine to include all WALs. However,
there might be some corner cases, where the backup storage is acting slow, as
mentioned in my previous comment. In a hypothetical case like that, it could be
possible that there's a significant difference in the
everything-up-to-this-point-is-backed-up between different tables. If the user
does some kind of cross-table operation, there's a higher change of data being
out of sync.
This is all very hypothetical, but keeping this check does avoid that
scenario. Hence why I'm in favor of keeping the check in the code.
--
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]
