Repository navigation
HBASE-29905 BackupLogCleaner: skip tables no longer in the backup set - #7761
janvanbesien wants to merge 1 commit into
Conversation
5a9a2ef to
9f69699
Compare
|
Hi Jan, 2 remarks regarding the format of the PR:
|
9f69699 to
81e9e0d
Compare
81e9e0d to
1f676bb
Compare
|
I've force-pushed a rebase on the current master. |
1f676bb to
5e8a415
Compare
DieterDePaepe
left a comment
DieterDePaepe
left a comment
There was a problem hiding this comment.
A few minor changes to do, looks good overall.
There was a problem hiding this comment.
I rebased your changes to the latest master, and there was some refactoring involved. Would appreciate a second pair of eyes to take a look at the new code.
There was a problem hiding this comment.
I fixed all your remarks. However, when trying to rebase on master, I noticed that the test seems to work now without any code changes. So either the problem has been fixed it in the mean time via another change, or my test is no good.
I pushed it like this (test only) but I should find some more time to deep dive.
There was a problem hiding this comment.
I did another rebase & update - but the original issue persists. Found no evidence the issue was fixed in your rebased branch. So I'm force pushing both test & code changes again.
There was a problem hiding this comment.
Two observations that I had while reviewing, but not something that needs fixing in this PR. Mainly thinking out loud, and might log issues for these.
- If a table is deleted, it will result in WAL files sticking around until all backups containing that deleted table are gone (because that is when the incremental backup set gets updated). If long backup retention times are used, this can still be problematic. There's room for further improvement here.
- If a table is deleted and recreated with the same name, the backup info will still have the timestamps of the deleted table, and WALs that are included in incremental backups will also contain data of the deleted table. This would lead to a corrupted restore.
There was a problem hiding this comment.
Fixed the first point as part of the updated PR.
Logged the second point as https://issues.apache.org/jira/browse/HBASE-30542
|
@hgromer Could you also review this PR? |
5e8a415 to
792959a
Compare
792959a to
f3a3000
Compare
…es in system:backup table BackupLogCleaner computes the WAL deletion boundary from the tableSetTimestampMap of the most recent backup per backup root. A backup fills this map from all trslm: rows of its backup root, so the map also contains tables that are no longer backed up, such as deleted tables or tables whose backups were all deleted. Nothing removes these trslm: rows, so their timestamps stop advancing. Because the boundary per region server is the minimum timestamp over all tables, a single stale table prevents the cleanup of all newer WALs on every region server. When computing the boundary, BackupLogCleaner now only considers tables that are both in the incremental backup set of the backup root and still exist: - The incremental backup set gets every table of a full backup, and backup deletion removes tables that no longer have any backup. - Incremental backups skip tables that no longer exist, so a deleted table no longer needs its WALs, even while backups containing it are retained. Co-authored-by: Dieter De Paepe <dieterdp@ngdata.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f3a3000 to
b80ca6d
Compare
BackupLogCleaner.serverToPreservationBoundaryTs() computes the WAL deletion boundary by iterating over tableSetTimestampMap from persisted BackupInfo sessions. This map can contain entries for tables that were once part of the backup set but have since had all their backups deleted. Their stale, old timestamps drag the minimum WAL boundary back, preventing old WALs from being cleaned up.
Fix: when computing boundaries, load the incrbackupset per backup root from BackupSystemTable and skip tables not in the active set. The incrbackupset is populated on every full backup and pruned when backups are deleted, so it accurately reflects which tables still need WAL retention.