[SPARK-15288] [Mesos] Mesos dispatcher should handle gracefully when any thread gets UncaughtException

[devaraj-kavali]

What changes were proposed in this pull request?

Adding the default UncaughtExceptionHandler to the MesosClusterDispatcher.

How was this patch tested?

I verified it manually, when any of the dispatcher thread gets uncaught exceptions then the default UncaughtExceptionHandler will handle those exceptions.

[tnachen]

ok to test

[SparkQA]

Test build #60999 has finished for PR 13072 at commit 2f306a7.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #70646 has finished for PR 13072 at commit 7f1c9f6.

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

[skonto]

@mgummelt @tnachen should we ask for a merge?

[tnachen]

LGTM, @srowen can you please take a look?

[srowen]

We only otherwise do this in Executor.scala. I'm trying to understand if this is the right thing to do? I don't doubt it, just don't know.

[devaraj-kavali]

MesosClusterDispatcher also has multiple threads like Executor, when any one thread terminates in the MesosClusterDispatcher process due to some error/exception it keeps running without performing the terminated thread functionality. I think we need to handle those uncaught exceptions from the MesosClusterDispatcher process threads using the UncaughtExceptionHandler and take the action instead of running the MesosClusterDispatcher without performing the functionality and without notifying the user.

[devaraj-kavali]

@srowen Can you check this?

[srowen]

I can't really evaluate the change; is anyone else familiar with Mesos around to comment? that'd be best.

[skonto]

LGTM. @srowen Ideally all processes we have should handle thread termination correctly, same applies to MesosClusterDispatcher. Btw I think the call in Executor.scala which sets the handler should be done in static code and ASAP, for example in MesosExecutorBackend this should happen in its main method. From what I see right now the call is executed when a new Executor class is instantiated which might be a bit late because for example the createExecutorEnv call which always comes first, creates its own threads (eg. MapoutTracker threads).

[mgummelt]

LGTM. @srowen please merge.

Out of curiosity, @devaraj-kavali what exception were you seeing?

[devaraj-kavali]

Thanks @mgummelt for the confirmation, It throws SparkException with the bug SPARK-15359/#13143.

[SparkQA]

Test build #3585 has finished for PR 13072 at commit 7f1c9f6.

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

[srowen]

Merged to master