HBASE-29210: Introduce Validation for PITR-Critical Backup Deletion #6848
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
vinayakphegde
force-pushed
the
HBASE-28993
branch
from
d292b73 to
2eb33d3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
anmolnar
requested changes
Apr 4, 2025
Contributor
anmolnar
left a comment
anmolnar
left a comment
There was a problem hiding this comment.
Overall lgtm. A few comments.
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // Check if another valid full backup exists for this table | ||
| long finalPitrMaxStartTime = pitrMaxStartTime; |
Contributor
There was a problem hiding this comment.
Why do you need to introduce this new variable?
Contributor
Author
There was a problem hiding this comment.
Otherwise we'll get this error: Variable used in lambda expression should be final or effectively final
Contributor
There was a problem hiding this comment.
I see. Make it final then.
final long finalPitrMaxStartTime = pitrMaxStartTime;
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
Outdated
Show resolved
Hide resolved
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.
anmolnar
approved these changes
Apr 8, 2025
| } | ||
|
|
||
| // Check if another valid full backup exists for this table | ||
| long finalPitrMaxStartTime = pitrMaxStartTime; |
Contributor
There was a problem hiding this comment.
I see. Make it final then.
final long finalPitrMaxStartTime = pitrMaxStartTime;
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
wchevreuil
reviewed
Apr 9, 2025
|
|
||
| // Check if another valid full backup exists for this table | ||
| List<BackupInfo> backupHistory = backupSystemTable.getBackupInfos(BackupState.COMPLETE); | ||
| final long finalPitrMaxStartTime = pitrMaxStartTime; |
Contributor
There was a problem hiding this comment.
nit: declare pitrMaxStartTime as a final MutableLong, then no need to use extra variable here.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
wchevreuil
approved these changes
Apr 10, 2025
anmolnar
pushed a commit
that referenced
this pull request
Jul 28, 2025
…6848) Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Jul 29, 2025
…pache#6848) Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Jul 29, 2025
…pache#6848) Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
anmolnar
pushed a commit
that referenced
this pull request
Sep 11, 2025
…6848) Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
anmolnar
pushed a commit
that referenced
this pull request
Nov 6, 2025
…6848) Signed-off-by: Andor Molnár <[email protected]> Signed-off-by: Wellington Chevreuil <[email protected]>
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 validation to prevent the deletion of backups that are essential for Point-In-Time Recovery (PITR). If a backup is the only remaining full backup enabling PITR for certain tables, deletion will be blocked unless the force option is explicitly specified.