-
Notifications
You must be signed in to change notification settings - Fork 9.1k
YARN-11509. The FederationInterceptor#launchUAM Added retry logic. #5727
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. |
💔 -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. |
.../main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TokenAndRegisterResponse.java
Show resolved
Hide resolved
|
||
// LaunchUAM And RegisterApplicationMaster | ||
try { | ||
|
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.
Avoid this break line.
FinishApplicationMasterResponse finishResponse = | ||
interceptor.finishApplicationMaster(finishReq); | ||
Assert.assertNotNull(finishResponse); | ||
Assert.assertEquals(true, finishResponse.getIsUnregistered()); |
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.
assertTrue
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 will modify the code.
💔 -1 overall
This message was automatically generated. |
import java.util.concurrent.ExecutorService; | ||
import java.util.concurrent.Executors; | ||
import java.util.concurrent.Future; | ||
import java.util.concurrent.*; |
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.
Avoid
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.
Thanks for helping to review the code, I will fix it.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Let's fix the checkstyle. |
💔 -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. |
🎊 +1 overall
This message was automatically generated. |
@goiri Can you help to merge this pr into the trunk branch? Thank you very much! |
@goiri Thank you very much for your help in reviewing the code! |
Description of PR
JIRA: YARN-11509. The FederationInterceptor#launchUAM Added retry logic.
There is a "todo" in the FederationInterceptor#registerAndAllocateWithNewSubClusters method. According to the "todo" description, the request needs to be retried to other subclusters, but changing the parameter requests in registerAndAllocateWithNewSubClusters is not a good operation. It is better to add retry logic here.
We don't need to worry about losing requests because when the request cannot be satisfied, the AM of the task will continue to apply, and these requests will be properly transferred to other clusters for execution.
How was this patch tested?
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?