-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-17597. [ARR] RouterSnapshot supports asynchronous rpc. #6994
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
Conversation
🎊 +1 overall
This message was automatically generated. |
return invokedResult.replaceFirst(loc.getDest(), loc.getSrc()); | ||
}); | ||
} | ||
return AsyncUtil.asyncReturn(String.class); |
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.
@LeoLeeeeee Hi, thank you for your contribution. I think AsyncUtil can be deleted here. You can use asyncReturn(String.class) directly.
asyncApply((ApplyFunction<Map<FederationNamespaceInfo, SnapshottableDirectoryStatus[]>, | ||
SnapshottableDirectoryStatus[]>) | ||
ret -> RouterRpcServer.merge(ret, SnapshottableDirectoryStatus.class)); | ||
return AsyncUtil.asyncReturn(SnapshottableDirectoryStatus[].class); |
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.
Same as above.
4e0b405
to
a591902
Compare
Hi, @LeoLeeeeee HDFS-17545 has already been merged into HDFS-17531, please rebase your development branch using HDFS-17531. |
591e648
to
428c52d
Compare
@KeeProMise Ok, recommit, please take a look again. |
💔 -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.
LGTM! @Hexiaoqiao Hi, if you have time, please help to take a look at this PR, thanks!
@LeoLeeeeee @KeeProMise Thanks for your works. LGTM. +1 from my side. One thing as #6983 mentioned, do we need to move async RPC implement under single package? Thanks. |
@Hexiaoqiao Thanks, I also have this idea. I will commit a PR in the future and put these PRs together with the previous PRs under an async package. |
make sense to me. +1. |
@LeoLeeeeee Thanks for your contribution! |
…Contributed by Wenqi Li. Reviewed-by: Jian Zhang <keepromise@apache.org> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
Description of PR
please see https://issues.apache.org/jira/browse/HDFS-17597
How was this patch tested?
new ut TestRouterAsyncSnapshot
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?