HDFS-17788. [ARR] getFileInfo not handle exception rightly which may cause FileNotFoundException in DistributedFileSystem #7703
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
|
Hi, @Hexiaoqiao @slfan1989 @KeeProMise . Could you please help review this PR when you have free time? Thanks a lot!!! |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
hfutatzhanghb
force-pushed
the
HDFS-17788
branch
2 times, most recently
from
0a2d3a1 to
adba37e
Compare
Hi @hfutatzhanghb LGTM! wait for the check to be approved. |
…cause FileNotFoundException in DistributedFileSystem.
@KeeProMise , Thanks very much for your reviewing!!! |
|
💔 -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. |
|
@KeeProMise have fixed some checkstyle problems. |
|
🎊 +1 overall
This message was automatically generated. |
|
Hi @hfutatzhanghb LGTM! Let's wait for two days to see if anyone else has comments. |
KeeProMise
approved these changes
May 23, 2025
slfan1989
approved these changes
May 26, 2025
|
@hfutatzhanghb Thanks for the contribution! @KeeProMise Thanks for the review! From my side, it's a +1, but I think it would be best to have @Hexiaoqiao help review this PR. |
Hexiaoqiao
reviewed
May 27, 2025
| @@ -579,6 +579,8 @@ public HdfsFileStatus getFileInfo(String src) throws IOException { | |||
| if (e instanceof NoLocationException | |||
| || e instanceof RouterResolveException) { | |||
| noLocationException[0] = e; | |||
| } else { | |||
There was a problem hiding this comment.
Great catch here. What about other RPCs invoke, if has the same issue? Thanks.
There was a problem hiding this comment.
@Hexiaoqiao , Thanks a lot for your reviewing. Have checked other asyncCatch function invocation, not found the same issue.
Hexiaoqiao
approved these changes
May 27, 2025
Hi, @Hexiaoqiao . Very nice advice, actually i have checked other asyncCatch function invoke in our production cluster and not found similar issue. |
|
Thank you for @hfutatzhanghb 's contributions and @slfan1989 @Hexiaoqiao 's review. merged! |
|
Thank you all !!! |
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
Refer to HDFS-17788.
When namenode failovers, getFileInfo rpc may return null accidentally which is wrong behavior. We should throw original exception and let client retry.
How was this patch tested?
Add an unit test.