-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29459 Capture bulkload files only till IncrCommittedWalTs during Incremental Backup #7166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kgeisz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
taklwu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a question to understand the logic, but otherwise LGTM
| } | ||
| result.add(new BulkLoad(table, region, fam, path, row)); | ||
| LOG.debug("found orig " + path + " for " + fam + " of table " + region); | ||
| LOG.debug("found orig {} for {} of table {} with timestamp {}", path, fam, region, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit adjust the short form to clear wording
| LOG.debug("found orig {} for {} of table {} with timestamp {}", path, fam, region, | |
| LOG.debug("found original path {} for column family {} of table {} with timestamp {}", path, fam, region, |
| long timestamp = 0L; | ||
| for (Cell cell : res.listCells()) { | ||
| row = CellUtil.cloneRow(cell); | ||
| timestamp = cell.getTimestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so is this timestamp the IncrCommittedWalTs? I cannot find other timestamp representing it but it looks like this PR share the same setup as #7150 that used the BulkLoad#timestamp as IncrCommittedWalTs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This timestamp is Bulkload timestamp (time at which Bulkload operation is triggered). This will be saved in backup system table and compared with IncrCommittedWalTs during incremental backup
please fix spotless, spotbugs, javac and blanks
| List<String> activeFiles = new ArrayList<>(); | ||
| List<String> archiveFiles = new ArrayList<>(); | ||
| List<BulkLoad> bulkLoads = backupManager.readBulkloadRows(tablesToBackup); | ||
| List<BulkLoad> bulkLoads = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| List<BulkLoad> bulkLoads = new ArrayList<>(); | |
| List<BulkLoad> bulkLoads = backupInfo.isContinuousBackupEnabled() ? | |
| backupManager.readBulkloadRows(tablesToBackup, backupInfo.getIncrCommittedWalTs()) : | |
| backupManager.readBulkloadRows(tablesToBackup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may help the spotbugs error
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…g Incremental Backup (#7166) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Reviewed by: Kevin Geiszler <kevin.j.geiszler@gmail.com>
…g Incremental Backup (apache#7166) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Reviewed by: Kevin Geiszler <kevin.j.geiszler@gmail.com>
…g Incremental Backup (apache#7166) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Reviewed by: Kevin Geiszler <kevin.j.geiszler@gmail.com>
…g Incremental Backup (#7166) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Reviewed by: Kevin Geiszler <kevin.j.geiszler@gmail.com>
…g Incremental Backup (#7166) Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Reviewed by: Kevin Geiszler <kevin.j.geiszler@gmail.com>
In incremental backup, we copy bulkload files to backup location (along with Hfiles generated from WALs). We introduced IncrCommittedWalTs field for incremental backup and this needs to be used in copy bulkload code so only bulkload files till IncrCommittedWalTs timestamp are copied to incremental backup
JIRA: https://issues.apache.org/jira/browse/HBASE-29459