hgromer commented on code in PR #8694:
URL: https://github.com/apache/hbase/pull/8694#discussion_r4171338130
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java:
##########
@@ -217,14 +251,6 @@ private List<String> getLogFilesForNewBackup(Map<String,
Long> olderTimestamps,
resultLogFiles.add(currentLogFile);
}
- // It is possible that a host in .oldlogs is an obsolete region server
- // so newestTimestamps.get(host) here can be null.
- // Even if these logs belong to a obsolete region server, we still need
- // to include they to avoid loss of edits for backup.
- Long newTimestamp = newestTimestamps.get(host);
Review Comment:
Hmm, okay I think I see your point, I didn't realize that WAL archival was
random. This comment makes sense. In that case, I think your proposed change
makes sense, while still preventing the data loss scenario that I'm trying to
address
--
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]
