HBASE-29984 Support separate old WAL directories in backup (#8512) - #8620
Open
taklwu wants to merge 1 commit into
Open
HBASE-29984 Support separate old WAL directories in backup (#8512)#8620taklwu wants to merge 1 commit into
taklwu wants to merge 1 commit into
Conversation
Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org> Co-authored-by: mazhengxuan <mazhengxuan@didiglobal.com>
There was a problem hiding this comment.
🟡 Changes recommended
The newly added test can be flaky due to creating a potentially-colliding archived WAL file path without explicitly overwriting/ensuring uniqueness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates HBase backup’s incremental WAL collection logic to handle hbase.separate.oldlogdir.by.regionserver, where archived WALs may be stored under per-RegionServer subdirectories, and adds/extends unit tests to validate the behavior.
Changes:
- Make incremental backup scan
.oldlogs/oldWALsrecursively so archived WALs in per-server subdirectories are discovered. - Enhance
BackupUtils.parseHostFromOldLogto derivehost:portfrom the parent directory when the parent is a fullServerName. - Add a new incremental backup manager test and extend existing parsing tests to cover per-server archived WAL layouts.
File summaries
| File | Description |
|---|---|
| hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupManager.java | New test validating incremental WAL collection for both flat and per-server old WAL directory layouts. |
| hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupUtils.java | Extends WAL host parsing test coverage for archived WALs placed under a server-name directory. |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java | Improves archived WAL host parsing by supporting per-server directory layouts and adds a recursive file listing helper. |
| hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java | Switches old WAL enumeration from non-recursive listStatus to recursive listing to support per-server old WAL directories. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Path archivedWAL = new Path(archiveDir, walName); | ||
| FileSystem fs = walRootDir.getFileSystem(conf1); | ||
| fs.mkdirs(archiveDir); | ||
| fs.create(archivedWAL).close(); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is a clean cherry-pick from 96922fc