Uh oh!
There was an error while loading. Please reload this page.
HBASE-29209: Implement Backup Cleanup Command to Remove Older WALs Not Required for PITR of Any Table - #6847
HBASE-29209: Implement Backup Cleanup Command to Remove Older WALs Not Required for PITR of Any Table#6847vinayakphegde wants to merge 4 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Apache-HBase
commented
Mar 26, 2025
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Mar 26, 2025
🎊 +1 overall
This message was automatically generated. |
| } else if (BackupCommand.CLEANUP.name().equalsIgnoreCase(cmd)) { | ||
| type = BackupCommand.CLEANUP; | ||
| } else { |
There was a problem hiding this comment.
I would name this something more specific, unless this command intends to clean up entries that may be left behind for full and incremental backups as well
| * The {@code CleanupCommand} class is responsible for removing Write-Ahead Log (WAL) and | ||
| * bulk-loaded files that are no longer needed for Point-in-Time Recovery (PITR). | ||
| * <p> | ||
| * The cleanup process follows these steps: | ||
| * <ol> | ||
| * <li>Identify the oldest full backup and its start timestamp.</li> | ||
| * <li>Delete WAL files older than this timestamp, as they are no longer usable for PITR with any | ||
| * backup.</li> | ||
| * </ol> |
There was a problem hiding this comment.
The standard approach in HBase is to delete old files via extensions of the BaseFileCleanerDelegate. For example, the BackupLogCleaner which already handles cleaning up WALs as they relate to backups.
These cleaners should be run by the HMaster's CleanerChore, which will ensure that we only delete files which live in the intersection of all cleaners' outputs. On top of that critical safety guarantee, this also has the advantage of being run periodically, automatically — for more sophisticated HBase operators, this is a critical advantage because manual operations for textbook operations like backups do not scale well.
There was a problem hiding this comment.
Thanks, @rmdmattingly, for the review comments!
I’d like to clarify that we are specifically cleaning up WALs in the backup location (e.g., S3, where they are continuously replicated), not the cluster’s WALs. If we were dealing with cluster WALs, your point would certainly apply—does that sound correct?
Regarding keeping this command manual:
- All other backup-related commands are currently manual.
- This command depends on the delete command. What we are doing here is identifying the first full backup in the system and deleting all WALs before that point.
- These WALs are needed for point-in-time recovery (PITR), where we replay WALs from the full backup up to the specified recovery point. If the full backup itself no longer exists, keeping those WALs serves no purpose.
- Since deleting full backups is already a manual operation, there is little benefit in automating this cleanup.
That said, we could explore the possibility of running this command periodically and automatically in future iterations.
Let me know your thoughts!
There was a problem hiding this comment.
Ah okay, thanks for the clarifications here. Maybe we could bake this clarification into the JavaDocs
You make some good points here, but I don't think they full take into account the variety of ways in which people deploy HBase.
All other backup-related commands are currently manual.
This is true to a large extent, but the non-emergency commands have at least been exposed in the Admin interface to make programmatic backups easily achievable. Maybe wiring up through the Admin is a fair compromise?
This command depends on the delete command. What we are doing here is identifying the first full backup in the system and deleting all WALs before that point.
If this operation can only follow a delete, and WALs are made useless by said delete, then should this operation just be a part of the backup deletion process?
Since deleting full backups is already a manual operation, there is little benefit in automating this cleanup.
I don't think it's true that backup deletions are necessarily manual from an operator's perspective. For example, a company backing up their data in S3 could be making use of bucket TTLs to clean up their old backups. In that case, it would be nice for unusable WALs to clean themselves up organically too.
There was a problem hiding this comment.
If this operation can only follow a delete, and WALs are made useless by said delete, then should this operation just be a part of the backup deletion process?
That's my point as well. We could do this cleanup as part of the backup delete command, in which case we don't need to deal with whether this should be automatic or manual.
There was a problem hiding this comment.
Thanks, @rmdmattingly and @anmolnar.
Okay, we can incorporate this cleanup process into the delete command itself.
Currently, the delete command is used to remove both full and incremental backups. We have now introduced a new validation for PITR-Critical Backup Deletion. Please check the PR here: #6848 and review it.
I will also add this cleanup logic at the end of the delete process to remove any WALs that can be deleted (which were previously retained due to this backup). How does that sound?
rmdmattingly
commented
Mar 26, 2025
I also want to say thank you for all of your work on the continuous backup idea! We have a homegrown solution at my company to achieve something similar, and I would love if we could stop maintaining that system upon adoption of your work here. |
vinayakphegde
commented
Mar 26, 2025
Sure, @rmdmattingly! Thanks for the review. We'll keep you involved and look forward to your input. :) |
anmolnar
commented
Mar 28, 2025
kgeisz
left a comment
There was a problem hiding this comment.
LGTM, although @rmdmattingly's change request has still not been addressed.
| */ | ||
| private void cleanupOldWALFiles(Configuration conf, String backupWalDir, long cutoffTime) | ||
| throws IOException { | ||
| System.out.println("Starting WAL cleanup in backup directory: " + backupWalDir |
There was a problem hiding this comment.
@vinayakphegde, are you using System.out.println() in this file so output can be displayed to the user when they use hbase command line commands?
| .println("Checking WAL directory: " + dirName + " (Start Time: " + dayStart + ")"); | ||
| // If WAL files of that day are older than cutoff time, delete them | ||
| if (dayStart + ONE_DAY_IN_MILLISECONDS - 1 < cutoffTime) { |
There was a problem hiding this comment.
@vinayakphegde, just curious, what is the purpose of adding ONE_DAY_IN_MILLISECONDS - 1? Is dayStart always at the beginning of the day and ONE_DAY_IN_MILLISECONDS - 1 moves the timestamp to the end of the day?
vinayakphegde
commented
Jun 13, 2025
Cleanup logic is integrated with Delete Command: https://issues.apache.org/jira/browse/HBASE-29255 |
This PR introduces the CleanupCommand, which is used in the removal of obsolete Write-Ahead Log (WAL) and bulk-loaded files to optimize storage usage in HBase's continuous backup system. The cleanup process identifies the oldest full backup timestamp and deletes WAL files older than this cutoff, ensuring Point-in-Time Recovery (PITR) is not compromised.