-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-13118. SnapshotDiffReport should provide the INode type. #1313
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: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Updated MR to fix findbugs and checkstyle issues. |
💔 -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.
Quickly reviewed it, and I think the patch makes sense to me and has high quality.
...s/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotDiffListingInfo.java
Outdated
Show resolved
Hide resolved
Forgot to commit SnapshotDiffListingInfo 😞 |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
...ect/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotDiffReport.java
Outdated
Show resolved
Hide resolved
...s/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotDiffListingInfo.java
Outdated
Show resolved
Hide resolved
Thanks @ehiggs for working on this. The patch overall looks good to me with few minor comments . The test failure TestWebHDFS#testWebHdfsSnapshotDiff looks related. Can you please check this as well? |
It was indeed related. I was not outputting the inodeType as part of the Json for WebHdfs. This should be fixed now. |
💔 -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. |
The changes look good to me. Can you please address the checkstyle issues? |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Tests that failed all appear to have done so with resource exhaustion:
|
/retest |
…umentation/specification as well.
💔 -1 overall
This message was automatically generated. |
Looks like this PR needs rebase. Other than that i think the patch is good to go if we can verify the failed tests aren't related. |
@ehiggs - Can you confirm if this PR is ready to get in? Also, can you verify that the failed |
No description provided.