tgravescs commented on code in PR #36952:
URL: https://github.com/apache/spark/pull/36952#discussion_r915919842
##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnRMClient.scala:
##########
@@ -93,10 +93,16 @@ private[spark] class YarnRMClient extends Logging {
* @param status The final status of the AM.
* @param diagnostics Diagnostics message to include in the final status.
*/
- def unregister(status: FinalApplicationStatus, diagnostics: String = ""):
Unit = synchronized {
- if (registered) {
- amClient.unregisterApplicationMaster(status, diagnostics,
uiHistoryAddress)
+ def unregister(status: FinalApplicationStatus,
+ conf: YarnConfiguration,
+ diagnostics: String = ""): Unit = synchronized {
+ if (!registered) {
+ amClient = AMRMClient.createAMRMClient()
Review Comment:
the question for me is, is this really needed vs possibly introducing other
issues. while it may be convenient, user can just go look in the log to find
the error. You tested a bunch of scenarios which is great, but I'm sure there
are others. The main one I mention here is what if registration failed or RM is
not responsive? Can you test out that scenario and see what happens?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
