[SPARK-3469] Make sure all TaskCompletionListener are called even with failures

[rxin]

This is necessary because we rely on this callback interface to clean resources up. The old behavior would lead to resource leaks.

Note that this also changes the fault semantics of TaskCompletionListener. Previously failures in TaskCompletionListeners would result in the task being reported immediately. With this change, we report the exception at the end, and the reported exception is a TaskCompletionListenerException that contains all the exception messages.

[rxin]

Jenkins, add to whitelist.

[rxin]

Jenkins, test this please.

[SparkQA]

QA tests have started for PR 2343 at commit 1cb444d.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2343 at commit 1cb444d.

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

[SparkQA]

QA tests have started for PR 2343 at commit 1cb444d.

• This patch merges cleanly.

[SparkQA]

QA tests have started for PR 2343 at commit 29b6162.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2343 at commit 1cb444d.

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

[SparkQA]

QA tests have finished for PR 2343 at commit 29b6162.

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

[shivaram]

Do we need to change the semantics though ? We could still save the first exception and return a task failure ?

[rxin]

What do you think would be a good way to handle this?

Perhaps throw back the first exception encountered? or throw an exception with all the error messages

[rxin]

Ok I pushed a new version that creates a TaskCompletionListenerException that contains all the error messages.

[SparkQA]

QA tests have started for PR 2343 at commit aa68ea4.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2343 at commit aa68ea4.

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

[shivaram]

update this comment

[shivaram]

I left a minor comment - Otherwise LGTM

[shivaram]

Jenkins, retest this please

[SparkQA]

QA tests have started for PR 2343 at commit ac5baea.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 2343 at commit ac5baea.

• This patch passes unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • throw new IllegalStateException("The main method in the given main class must be static")
  • class TaskCompletionListenerException(errorMessages: Seq[String]) extends Exception

[rxin]

Ok merging this in master. Thanks for taking a look.