-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-17650. [ARR] The router server-side rpc protocol PB supports asynchrony. #7139
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 a 8000 nd privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@Hexiaoqiao @KeeProMise Sir, codes mainly from HDFS-17531. PTAL when you are free. Thanks a lot. |
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 After a brief look, please add some UT, you can refer to HDFS-17544. [ARR] The router client rpc protocol PB supports asynchrony.
private final static RefreshUserToGroupsMappingsResponseProto | ||
|
||
protected final static RefreshUserToGroupsMappingsResponseProto | ||
VOID_REFRESH_USER_GROUPS_MAPPING_RESPONSE = | ||
RefreshUserToGroupsMappingsResponseProto.newBuilder().build(); | ||
|
||
private final static RefreshSuperUserGroupsConfigurationResponseProto | ||
protected final static RefreshSuperUserGroupsConfigurationResponseProto | ||
VOID_REFRESH_SUPERUSER_GROUPS_CONFIGURATION_RESPONSE = |
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 thanks for your contribution! It is recommended not to modify common because the pipeline will run for a long time. There are only 2 variables here, you can copy them directly to RouterRefreshUserMappingsProtocolServerSideTranslatorPB
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.
@KeeProMise Sir , thanks for reminding. fixed
public static <T> void asyncRouterServer(ServerReq<T> req, ServerRes<T> res) { | ||
final ProtobufRpcEngineCallback2 callback = | ||
ProtobufRpcEngine2.Server.registerForDeferredResponse2(); | ||
|
||
CompletableFuture<Object> completableFuture = | ||
CompletableFuture.completedFuture(null); | ||
completableFuture.thenCompose(o -> { |
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.
It's better to add some comments
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.
done.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@KeeProMise Thanks for your suggestions. Will add soonly, |
💔 -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. |
df6a5b6
to
ea3c4c8
Compare
2a50694
to
b5020d7
Compare
💔 -1 overall
This message was automatically generated. |
b5020d7
to
d1355d2
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
c5e833f
to
15e4c0e
Compare
💔 -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. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@Hexiaoqiao Hi, if you have time, please help to take a look at this PR, thanks! The failed unit test is not related to this PR. It runs successfully on my local machine. |
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.
I didn't get why we need to update ClientNamenodeProtocolServerSideTranslatorPB
and NamenodeProtocolServerSideTranslatorPB
. Others look good to me. Will give +1 from my side once solve these suspicious changes. Thanks.
@Hexiaoqiao Sir, we just make some fields and methods from private to protected in |
Got it. Make sense to me. +1. |
@hfutatzhanghb thanks for your contributions, @Hexiaoqiao thanks for your review! merged! |
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…nchrony. (apache#7139). Contributed by hfutatzhanghb. Co-authored-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
Description of PR
The router server-side rpc protocol PB supports asynchrony.
How to test
Added TestRouterAsyncRpc and TestRouterAsyncRpcMultiDestination tests for integration testing the asynchronous router functionality.