-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29598 Fix regionDir in clone snapshots wfw to clone all mob files rightly t… #7309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a865ce7
to
6158621
Compare
…es rightly to mobdir
6158621
to
8f9187a
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue with clone snapshots where MOB (Medium Object) files were not being cloned correctly to the mob directory. The fix addresses the regionDir parameter used in the cloning process and ensures MOB regions use the default store file tracker.
- Fixed regionDir reference in RestoreSnapshotHelper to use the correct directory path for filesystem operations
- Added special handling for MOB regions to use the default store file tracker instead of the configured one
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
RestoreSnapshotHelper.java | Corrected regionDir parameter to fix MOB file cloning during snapshot restore |
StoreFileTrackerFactory.java | Added MOB region detection and default tracker assignment for MOB operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if ( | ||
ctx != null && ctx.getRegionInfo().getEncodedName() | ||
.equals(MobUtils.getMobRegionInfo(ctx.getTableName()).getEncodedName()) | ||
) { |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The nested method calls and multi-line condition check is hard to read. Consider extracting this MOB region check into a separate method like isMobRegion(StoreContext ctx)
to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
ctx != null && ctx.getRegionInfo().getEncodedName() | ||
.equals(MobUtils.getMobRegionInfo(ctx.getTableName()).getEncodedName()) |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential NullPointerException if ctx.getRegionInfo()
or ctx.getTableName()
returns null. Add null checks for these nested method calls or ensure they cannot be null in this context.
ctx != null && ctx.getRegionInfo().getEncodedName() | |
.equals(MobUtils.getMobRegionInfo(ctx.getTableName()).getEncodedName()) | |
ctx != null | |
&& ctx.getRegionInfo() != null | |
&& ctx.getTableName() != null | |
&& ctx.getRegionInfo().getEncodedName() | |
.equals(MobUtils.getMobRegionInfo(ctx.getTableName()).getEncodedName()) |
Copilot uses AI. Check for mistakes.
…o mobdir