hgromer commented on code in PR #7717:
URL: https://github.com/apache/hbase/pull/7717#discussion_r2800199310
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java:
##########
@@ -116,34 +115,55 @@ public static class BackupBoundariesBuilder {
private final Map<Address, Long> boundaries = new HashMap<>();
private final long tsCleanupBuffer;
- private long oldestStartCode = Long.MAX_VALUE;
+ private long oldestStartTs = Long.MAX_VALUE;
private BackupBoundariesBuilder(long tsCleanupBuffer) {
this.tsCleanupBuffer = tsCleanupBuffer;
}
- public BackupBoundariesBuilder addBackupTimestamps(String host, long
hostLogRollTs,
- long backupStartCode) {
- Address address = Address.fromString(host);
- Long storedTs = boundaries.get(address);
- if (storedTs == null || hostLogRollTs < storedTs) {
- boundaries.put(address, hostLogRollTs);
+ /**
+ * Updates the boundaries based on the provided backup info.
+ * @param backupInfo the most recent completed backup info for a backup
root, or if there is no
+ * such completed backup, the currently running backup.
+ */
+ public void update(BackupInfo backupInfo) {
+ switch (backupInfo.getState()) {
+ case COMPLETE:
+ // If a completed backup exists in the backup root, we want to
protect all logs that
+ // have been created since the log-roll that happened for that
backup.
+ for (TableName table :
backupInfo.getTableSetTimestampMap().keySet()) {
+ for (Map.Entry<String, Long> entry :
backupInfo.getTableSetTimestampMap().get(table)
+ .entrySet()) {
+ Address regionServerAddress = Address.fromString(entry.getKey());
+ Long logRollTs = entry.getValue();
+
+ Long storedTs = boundaries.get(regionServerAddress);
+ if (storedTs == null || logRollTs < storedTs) {
+ boundaries.put(regionServerAddress, logRollTs);
Review Comment:
Side note. I've noticed that the current implementation of BackupBoundaries
can lead to [problems](https://issues.apache.org/jira/browse/HBASE-29895) due
to `boundaries` being a global map.
I lean towards having a separate PR to address those issues, but if you'd
prefer we could group together both changes in the same patch
--
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]
