-
Notifications
You must be signed in to change notification settings - Fork 9k
HDFS-17543. [ARR] AsyncUtil makes asynchronous code more concise and easier. #6868
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
async util & ut async util & ut async util javadoc async util javadoc asyncTry feat test class javadoc test Finally & currentMethod
🎊 +1 overall
This message was automatically generated. |
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.CompletionException; | ||
|
||
public class AsyncForEachRun<I, T, R> implements AsyncRun<R> { |
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 Hi, sir. It's better to add some commets here to explain what this Class can do and I,T,R
generic type information.
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.
@hfutatzhanghb Thanks for your suggestion, I will add javadoc for these classes
@goiri @simbadzina @Hexiaoqiao @sjlee If you have time, please help to review this PR, thank you! |
💔 -1 overall
This message was automatically generated. |
This reverts commit 225106c.
💔 -1 overall
This message was automatically generated. |
This reverts commit 7909041.
💔 -1 overall
This message was automatically generated. |
I don't know why the compilation fails. |
🎊 +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. |
@goiri @simbadzina @Hexiaoqiao @sjlee @ayushtkn Hi, if everyone has some time, please take a look at this PR, thank you very much! |
Thanks for the contribution, @KeeProMise. One question: is there a reason this should be in hdfs as opposed to hadoop common? This feels like a generic utility that is not specific to hdfs. |
Hi, @sjlee thank you for your reply. In my opinion, it is indeed more reasonable to put it in hadoop common, but I am not sure whether other modules of hadoop need this package, so I put it in the hdfs package. If everyone agrees that the common package is better, I will move it under common. |
* | ||
* @param <R> the type of the result of the asynchronous operation | ||
*/ | ||
public interface FinallyFunction<R> { |
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, it's better to add @FunctionalInterface
annotation here and other place like here.
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.
@hfutatzhanghb Thank you for your careful review, I will add this later.
🎊 +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.
LGTM. +1 from my side. Let's wait two days if any folks would like to be involved here.
Thanks for your review! |
*/ | ||
public class SyncClass implements BaseClass{ | ||
private static final Logger LOG = | ||
LoggerFactory.getLogger(TestAsyncUtil.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.
This field seems never been used.
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!
*/ | ||
public class AsyncClass extends SyncClass{ | ||
private static final Logger LOG = | ||
LoggerFactory.getLogger(TestAsyncUtil.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.
TestAsyncUtil.class or AsyncClass.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.
done!
@KeeProMise Sir, leave two comments, others LGTM. |
Thanks for your review! |
💔 -1 overall
This message was automatically generated. |
Committed to HDFS-17531. Thanks @KeeProMise and @hfutatzhanghb . BTW. The failed unit test is not related to this PR. |
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
…easier. (apache#6868). Contributed by Jian Zhang. Reviewed-by: hfutatzhanghb <hfutzhanghb@163.com> Signed-off-by: He Xiaoqiao <hexiaoqiao@apache.org>
Description of PR
please see: https://issues.apache.org/jira/browse/HDFS-17543
NOTE: This is a sub-pull request (PR) related to HDFS-17531(Asynchronous router RPC). For more details or context, please refer to the main issue HDFS-17531
More detailed documentation: HDFS-17531 Router asynchronous rpc implementation.pdf and Aynchronous router.pdf
How was this patch tested?
new UT TestAsyncUtil
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?