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
HBASE-23202 ExportSnapshot (import) will fail if copying files to roo… #1791
Conversation
🎊 +1 overall
This message was automatically generated. |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
Outdated
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Test failure is a flaky, HBASE-24429 |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotFileCache.java
Outdated
Show resolved
Hide resolved
...e-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java
Show resolved
Hide resolved
...e-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java
Show resolved
Hide resolved
hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java
Show resolved
Hide resolved
6f1a191
to
d42f462
Compare
@z-york @ndimiduk I updated the patch based on your comments. Here is a summary of the changes:
Can you review the changes? Thanks. p.s., The current state is that hfile cleaner and snapshot wont step on each other. The only missing part is ExportSnapshot. However, there is an issue in ExportSnapshot which does not honor the case that working dir/root dir are from different filesystem. |
🎊 +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.
Thanks for making those changes, just a few lingering things.
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotHFileCleaner.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
Show resolved
Hide resolved
...e-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java
Show resolved
Hide resolved
...e-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotHFileCleaner.java
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
d42f462
to
4375fec
Compare
@z-york I uploaded a new patch which addresses some of your comments, put up explain for rest of your comments. Please let me know if there is anything missing, thanks. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Looks good to me. Thanks for the changes. Let's wait to see if @ndimiduk has any further comments. |
Hi @ndimiduk, can you review the latest patch to see if there is anything missing? Thanks! |
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.
Looks good. A couple comments about tests.
hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCache.java
Outdated
Show resolved
Hide resolved
...va/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCacheWithDifferentWorkingDir.java
Outdated
Show resolved
Hide resolved
...va/org/apache/hadoop/hbase/master/snapshot/TestSnapshotFileCacheWithDifferentWorkingDir.java
Show resolved
Hide resolved
…t directory takes longer than cleaner TTL
4375fec
to
ede033b
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
…t directory takes longer than cleaner TTL