[YARN][SPARK-4929] Bug fix: fix the yarn-client code to support HA

[SaintBacchus]

Nowadays, yarn-client will exit directly when the HA change happens no matter how many times the am should retry.
The reason may be that the default final status only considerred the sys.exit, and the yarn-client HA cann't benefit from this.
So we should distinct the default final status between client and cluster, because the SUCCEEDED status may cause the HA failed in client mode and UNDEFINED may cause the error reporter in cluster when using sys.exit.

[SparkQA]

Test build #24726 has finished for PR 3771 at commit 0e69924.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SaintBacchus]

@andrewor14 can you go through this problem? Thx.

[JoshRosen]

Also, /cc @tgravescs, another one of our YARN maintainers.

[SaintBacchus]

@tgravescs can you hava a look at this problem?

[tgravescs]

Can you please be a bit more specific and detail out exact what happens here? Are you referring to when RM has to failover or during rolling upgrade. Is the container brought down and then back up again... please just describe the scenario and what exactly is happening.

[SaintBacchus]

what @tgravescs says is close to the scenario, but it happens during the RM recover after broke down.

            if (finalStatus == FinalApplicationStatus.SUCCEEDED || isLastAttempt) {
              unregister(finalStatus, finalMsg)
              cleanupStagingDir(fs)
            }

In the code, it won't check the isLastAttempt if the finalStatus was FinalApplicationStatus.SUCCEEDED .
When the RM recovering happens, it would not check the isLastAttempt since the yarn-client had no chance to change the value of finalStatus. It's going to the unregister and this application can't recover itself.
So the yarn-client can't support the RM HA now.(yarn-cluster is OK)
And dividing the finalStatus into two parts is an easy way to avoid this problem and compatible with previous design.

[tgravescs]

@SaintBacchus so I'm still a bit unclear of the exact scenario. I just want to make sure we are handling everything properly so want to make sure I understand fully.

So this is when the RM goes down and is being brought back up or fails over to a standby. At that point it restarts the applications to start a new attempt. The shutdown hook is run and the code you mention above runs and unregisters. I understand client mode can't set it because spark context is not in the same process. The thing that is unclear to me is how is cluster mode setting the finalStatus to something other then succeeded? Is sparkContext being signalled and then throwing exception so that startUserClass catches it and marks it as failed?

[tgravescs]

I assume we are hitting the logic on line 108 above in if (!finished) {... I think that comment and code is based on the final status defaulting to success. In the very least we should update that comment explaining what is going to happen in client vs cluster mode. Since the DisassociatedEvent exits with success for client mode I think making the default as undefined for client mode is fine.

[SaintBacchus]

@tgravescs your comment is much more clear than what I said, I have use it instead of mine.Thx.

when Yarn HA event happens, the previous ApplicationMaster will throw a

java.io.IOException: Failed on local exception: java.io.EOFException

So the yarn cluster the catch the exception and change the final status.

But the yarn client will directly go into the ShutDownHook and cause the problem.
I think it haven't go into the DisassociatedEvent yet, because the driver is still alive.

[SparkQA]

Test build #25021 has finished for PR 3771 at commit c02bfcc.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #25154 has finished for PR 3771 at commit c02bfcc.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[tgravescs]

thanks @SaintBacchus the changes look good.