HBASE-29025: Enhance the full backup command to support Continuous Backup #6710
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
anmolnar
approved these changes
Feb 24, 2025
Comment on lines
+350
to
+354
| if (continuousBackup && !BackupType.FULL.equals(backupType)) { | ||
| System.out.println("ERROR: Continuous backup can Only be specified for Full Backup"); | ||
| printUsage(); | ||
| throw new IOException(INCORRECT_USAGE); | ||
| } |
Contributor
There was a problem hiding this comment.
I'm just wondering, since continuous backup is meaningful only for full backups, what if we introduce a new backup type? Maybe we already discussed this, I can't remember.
Usage: hbase backup create <type> <backup_path> [options]
type "full" to create a full backup image
"incremental" to create an incremental backup image
"continuous" to create new continuous backup
backup_path Full path to store the backup image
Contributor
Author
There was a problem hiding this comment.
Yeah, we discussed this and agreed to go with a new flag for the full backup.
- A continuous backup just takes a full backup and starts the WAL replication process. This happens only once—after that, we only need to take full backups. So, I don’t see a need for a separate backup type.
- Also, introducing a new type would unnecessarily differentiate full and continuous backups.
- In the future, we want to phase out the old incremental backup implementation. The plan is to use WALs replicated through continuous backup for incremental backups. So eventually, we might just have FULL and INCREMENTAL backups, where incremental backups rely on continuously replicated WALs.
- From a code perspective, our backup code has a lot of(almost everywhere) if-else checks for backup types. Adding a new type would mean modifying all of them.
What do you think?
Contributor
There was a problem hiding this comment.
Okay, I'm convinced let's go with this. I didn't remember the discussion.
Comment on lines
+152
to
+155
| if (backupInfo.isContinuousBackupEnabled()) { | ||
| handleContinuousBackup(admin); | ||
| } else { | ||
| handleNonContinuousBackup(admin); |
Contributor
There was a problem hiding this comment.
If we introduced new backup type for continuous backup, you would be able to create a brand new class just for the continuous backup potententially sharing some code if needed.
This if-else branch is a code smell I believe.
Contributor
There was a problem hiding this comment.
Otherwise code refactoring looks good to me.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| this.continuousBackupEnabled = continuousBackupEnabled; | ||
| } | ||
|
|
||
| public boolean getContinuousBackupEnabled() { |
Contributor
There was a problem hiding this comment.
NIT: if the return type is boolean the method name should be isContinousBackupEnabled
| String OPTION_ENABLE_CONTINUOUS_BACKUP = "cb"; | ||
| String LONG_OPTION_ENABLE_CONTINUOUS_BACKUP = "continuous-backup-enabled"; | ||
| String OPTION_ENABLE_CONTINUOUS_BACKUP_DESC = | ||
| "Flag indicating that the full backup " + "is part of a continuous backup process."; |
Contributor
There was a problem hiding this comment.
NIT: why + in between?
Contributor
Author
There was a problem hiding this comment.
We'll remove that
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
anmolnar
pushed a commit
that referenced
this pull request
Mar 25, 2025
…ckup (#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
anmolnar
pushed a commit
that referenced
this pull request
Jul 28, 2025
…ckup (#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Jul 29, 2025
…ckup (apache#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Jul 29, 2025
…ckup (apache#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
anmolnar
pushed a commit
that referenced
this pull request
Sep 11, 2025
…ckup (#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
anmolnar
pushed a commit
that referenced
this pull request
Nov 6, 2025
…ckup (#6710) * HBASE-29025: Enhance the full backup command to support continuous backup * add new check for full backup command regards to continuous backup flag * minor fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new parameter/option
-cb(--continuous-backup-enabled) for full backups. When specified, it enables continuous backup for the selected tables along with the full backup.Continuous backup leverages the HBase replication framework and uses
org.apache.hadoop.hbase.backup.replication.ContinuousBackupReplicationEndpoint(by default) to continuously stream WAL entries to the backup WAL directory defined byhbase.backup.continuous.wal.dir.