HDFS-17733. [ARR] Optimize isMultiDestDirectory method using AsyncUtil class #7415
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hfutatzhanghb
changed the title
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@Hexiaoqiao @KeeProMise Sir, cc when you have free time, thanks. |
KeeProMise
reviewed
Feb 21, 2025
| @Override | ||
| public boolean isMultiDestDirectory(String src) throws IOException { | ||
| try { | ||
| asyncComplete(false); |
There was a problem hiding this comment.
Hi, @hfutatzhanghb don’t need to add this here. If the code inside the asyncTry block executes without any exceptions, a CompletableFuture containing true or false will definitely be set to the thread-local variable. If the asyncTry block throws an UnresolvedPathException, it can be caught by asyncCatch and a CompletableFuture containing false will be set to the thread-local variable. If the asyncTry block throws any other exception, a CompletableFuture containing that exception will be set to the thread-local variable for the outer method to handle, which is consistent with the original synchronous logic.
There was a problem hiding this comment.
@KeeProMise very nice suggestion, thanks for reviewing. have fixed.
hfutatzhanghb
force-pushed
the
HDFS-17733
branch
from
718d1c2 to
aaa360e
Compare
hfutatzhanghb
force-pushed
the
HDFS-17733
branch
from
aaa360e to
b4ffc60
Compare
|
💔 -1 overall
This message was automatically generated. |
KeeProMise
reviewed
Feb 21, 2025
.../server/federation/router/async/TestRouterAsyncRPCMultipleDestinationMountTableResolver.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@slfan1989 Hi,Sir. Build met below problem when i add java document to class. It is the same with #7280 . Maybe we should fix this build problem. What's your opinons? Thanks. Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.0.1:javadoc-no-fork (default-cli) on project hadoop-hdfs-rbf: An error has occurred in Javadoc report generation:
[ERROR] Exit code: 1 - javadoc: warning - The code being documented uses modules but the packages defined in https://docs.oracle.com/javase/8/docs/api/ are in the unnamed module. |
|
💔 -1 overall
This message was automatically generated. |
@hfutatzhanghb Thank you for the feedback on this issue. I will continue to follow up on it. I have encountered this before, and it was fixed in the last 1-2 days. Personally, I feel it might be due to the ARR feature merge, but this should be a minor issue. |
Okay, Sir. And i will attempt to remove the javadoc of class TestRouterAsyncRPCMultipleDestinationMountTableResolver and see whether it can build successfully or not. Thanks a lot. |
|
@hfutatzhanghb I noticed the following error |
Oh. Got it. Thanks a lot. Sir. |
should remove the comment. |
@KeeProMise Thanks sir, have fixed. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
KeeProMise
approved these changes
Feb 24, 2025
There was a problem hiding this comment.
@hfutatzhanghb LGTM!
Hi, @Hexiaoqiao if you have time, please help to take a look at this PR, thanks!
Hexiaoqiao
reviewed
Feb 25, 2025
.../server/federation/router/async/TestRouterAsyncRPCMultipleDestinationMountTableResolver.java
Show resolved
Hide resolved
.../server/federation/router/async/TestRouterAsyncRPCMultipleDestinationMountTableResolver.java
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
Hi, @Hexiaoqiao |
|
Committed to trunk. Thanks @hfutatzhanghb . |
YanivKunda
pushed a commit
to YanivKunda/hadoop
that referenced
this pull request
Mar 23, 2025
…l class (apache#7415). Contributed by hfutatzhanghb. Reviewed-by: Jian Zhang <[email protected]> Signed-off-by: He Xiaoqiao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
Optimize isMultiDestDirectory method using AsyncUtil class
How was this patch tested?
add unit tests.