-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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-17601. [ARR] RouterRpcServer supports asynchronous rpc. #7108
Conversation
💔 -1 overall
This message was automatically generated. |
}); | ||
}); | ||
|
||
asyncCatch((AsyncCatchFunction<T, IOException>)(ret, ex) -> { |
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.
Should use CatchFunction.
throw ioe; | ||
} | ||
nss.removeIf(n -> n.getNameserviceId().equals(nsId)); | ||
invokeOnNs(method, clazz, io, nss); |
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.
Hi @hfutatzhanghb should use invokeOnNsAsync.
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, fixed.
Will add UTs these days. |
💔 -1 overall
This message was automatically generated. |
Hi @hfutatzhanghb And fix the blanks. |
💔 -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. |
@hfutatzhanghb LGTM. @Hexiaoqiao If you have time, please take a look. |
💔 -1 overall
This message was automatically generated. |
@hfutatzhanghb @KeeProMise Thanks for your works. It's better to add another unit test to cover it and please fix checkstyle first. Thanks. |
💔 -1 overall
This message was automatically generated. |
Hi @hfutatzhanghb Please check the checkstyle and there is one unused import - org.apache.hadoop.hdfs.server.federation.router.RouterRpcClient.isExpectedClass. Thanks. |
@Hexiaoqiao Thanks sir. have fixed. |
💔 -1 overall
This message was automatically generated. |
@hfutatzhanghb Hi, does this PR need to add a UT? |
🎊 +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.
LGMT. +1. Thanks.
@hfutatzhanghb Thanks for your contribution, and @Hexiaoqiao thanks for your review. |
Description of PR
RouterRpcServer supports asynchronous RPC, the following methods need to be transformed to asynchronous versions: