Skip to content

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented Feb 24, 2016

What changes were proposed in this pull request?

TaskContext supports task completion callback, which gets called regardless of task failures. However, there is no way for the listener to know if there is an error. This patch adds a new listener that gets called when a task fails.

How was the this patch tested?

New unit test case and integration test case covering the code path

@yhuai
Copy link
Contributor

yhuai commented Feb 24, 2016

test this pleaes

@SparkQA
Copy link

SparkQA commented Feb 24, 2016

Test build #2577 has finished for PR 11340 at commit 8c6f4c0.

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

@SparkQA
Copy link

SparkQA commented Feb 24, 2016

Test build #51862 has finished for PR 11340 at commit 9efd83c.

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

@SparkQA
Copy link

SparkQA commented Feb 24, 2016

Test build #51865 has finished for PR 11340 at commit a53f780.

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

@SparkQA
Copy link

SparkQA commented Feb 24, 2016

Test build #2579 has finished for PR 11340 at commit a53f780.

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

@SparkQA
Copy link

SparkQA commented Feb 25, 2016

Test build #2582 has finished for PR 11340 at commit a53f780.

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

private[spark] def markTaskFailed(error: Throwable): Unit = {
val errorMsgs = new ArrayBuffer[String](2)
// Process complete callbacks in the reverse order of registration
onFailureCallbacks.reverse.foreach { listener =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing: Should we store these callback as reversed order, so don't need to reverse them for every callback, especially for onCompleteCallbacks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how much it matters at all since it is an one time only operation and usually the number of callbacks is in the range of 0 to 2. Prepending efficiently would require us to not use the arraybuffer too.

* executing the callback in TaskCompletionListener.
*/
private[spark]
class TaskCompletionListenerException(errorMessages: Seq[String]) extends Exception {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was moved into JavaTaskCompletionListenerImpl as a static class.

@rxin
Copy link
Contributor Author

rxin commented Feb 26, 2016

@davies I updated the patch.

@SparkQA
Copy link

SparkQA commented Feb 26, 2016

Test build #52027 has finished for PR 11340 at commit b4630a0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class TaskCompletionListenerException(

@davies
Copy link
Contributor

davies commented Feb 26, 2016

LGTM, merging into master, thanks!

@asfgit asfgit closed this in 391755d Feb 26, 2016
davies pushed a commit to davies/spark that referenced this pull request Mar 2, 2016
    ## What changes were proposed in this pull request?

    TaskContext supports task completion callback, which gets called regardless of task failures. However, there is no way for the listener to know if ther
e is an error. This patch adds a new listener that gets called when a task fails.

    ## How was the this patch tested?
    New unit test case and integration test case covering the code path

    Author: Reynold Xin <[email protected]>

    Closes apache#11340 from rxin/SPARK-13465.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants