-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
HDFS-16954. RBF: rename a dir that has multiple destinations to a dir that just… #5483
Conversation
💔 -1 overall
This message was automatically generated. |
...rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAllResolver.java
Outdated
Show resolved
Hide resolved
...-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java
Outdated
Show resolved
Hide resolved
...rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAllResolver.java
Outdated
Show resolved
Hide resolved
💔 -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.
The test class TestRouterAllResolver.java
doesn't look like the correct place for this scenario to me
The Javadoc for the class says
/**
* Tests the use of the resolvers that write in all subclusters from the
* Router. It supports:
* <li>HashResolver
* <li>RandomResolver.
*/
And this is testing rename RPC, Can we explore adding a test in TestRouterRpcMultiDestination
or in TestRouterRPCMultipleDestinationMountTableResolver
...-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterClientProtocol.java
Outdated
Show resolved
Hide resolved
@ayushtkn I try to add this test in
|
Can try with TestRouterRPCMultipleDestinationMountTableResolver that is a specific varient without MockResolver |
@ayushtkn Thank you for your review. I move this test to TestRouterRPCMultipleDestinationMountTableResolver. |
🎊 +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. |
💔 -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.
Changes LGTM
...hadoop/hdfs/server/federation/router/TestRouterRPCMultipleDestinationMountTableResolver.java
Outdated
Show resolved
Hide resolved
这是自动回复邮件。来件已接收,谢谢。
|
💔 -1 overall
This message was automatically generated. |
Requires a rebase for a green build due to that cyclonedx change, can you rebase once |
…as a single destination
d5ca17a
to
981ae72
Compare
Done. Rebase it. Please review it again. thx. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
It appears that the compilation failure is not related to this patch. @ayushtkn |
🎊 +1 overall
This message was automatically generated. |
In case interested the reason for compilation failure was due to INFRA-24478 |
Description of PR
https://issues.apache.org/jira/browse/HDFS-16954
How was this patch tested?
TestRouterAllResolver
For code changes:
change in
rename
,rename2