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
YARN-11275. [Federation] Add batchFinishApplicationMaster in UAMPoolManager. #4792
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Map<String, FinishApplicationMasterResponse> responseMap = new HashMap<>(); | ||
Set<String> subClusterIds = this.unmanagedAppMasterMap.keySet(); | ||
|
||
if (subClusterIds.size() > 0) { |
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.
!isEmpty()
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.
Thank you very much for helping to review the code, I will modify the code.
FinishApplicationMasterResponse finshResponse = | ||
interceptor.finishApplicationMaster(finishReq); | ||
Assert.assertNotNull(finshResponse); | ||
Assert.assertEquals(true, finshResponse.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. |
@goiri Please help to review the code again, Thank you very much! |
LOG.info("Sending finish application request to RM {}", subClusterId); | ||
FinishApplicationMasterResponse uamResponse = null; | ||
try { | ||
uamResponse = finishApplicationMaster(subClusterId, request); |
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.
try {
FinishApplicationMasterResponse uamResponse = finishApplicationMaster(subClusterId, request);
return Collections.singletonMap(subClusterId, uamResponse);
} catch (Throwable e) {
LOG.warn("Failed to finish unmanaged application master: RM address: {} ApplicationId: {}",
subClusterId, appId, e);
return Collections.singletonMap(subClusterId, null);
}
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 your suggestion, the code looks very good, I will modify it.
finishReq.setTrackingUrl(""); | ||
finishReq.setFinalApplicationStatus(FinalApplicationStatus.SUCCEEDED); | ||
|
||
FinishApplicationMasterResponse finshResponse = |
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.
Single line?
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 fix it.
🎊 +1 overall
This message was automatically generated. |
@goiri Please help to review the code again, Thank you very much! |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@goiri Please help to review the code again, Thank you very much! |
🎊 +1 overall
This message was automatically generated. |
JIRA:YARN-11275. [Federation] Add batchFinishApplicationMaster in UAMPoolManager.