You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hvanhovell <gi...@git.apache.org> on 2017/03/10 15:02:13 UTC

[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

GitHub user hvanhovell opened a pull request:

    https://github.com/apache/spark/pull/17244

    [SPARK-19889][SQL] Make TaskContext callbacks thread safe

    ## What changes were proposed in this pull request?
    It is sometimes useful to use multiple threads in a task to parallelize tasks. These threads might register some completion/failure listeners to clean up when the task is completed/interrupted. We currently cannot register such a callback and be sure that it will get called, because the context might be in the process of invoking its callbacks, when the the callback gets registered.
    
    This PR improves this by synchronizing on the context itself when callbacks are invoked or added. This allows you to write the following code, and be guaranteed that the callback is invoked:
    ```scala
    val ctx: TaskContext = ...
    ctx.synchronized {
      if (!ctx.isCompleted) {
        cts.addTaskCompletionListener { _ =>
          // Some clean-up logic.
        }
      }
    }
    ```
    
    ## How was this patch tested?
    Existing tests. Adding a test without significant overhead is non-trivial and make testing flaky.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hvanhovell/spark SPARK-19889

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/17244.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #17244
    
----
commit d16ad88d83351d8c514a389cf2b65605ee558b11
Author: Herman van Hovell <hv...@databricks.com>
Date:   2017-03-10T14:52:16Z

    Make TaskContext callbacks threadsafe.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r106363391
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -52,62 +63,79 @@ private[spark] class TaskContextImpl(
       @volatile private var interrupted: Boolean = false
     
       // Whether the task has completed.
    -  @volatile private var completed: Boolean = false
    +  private var completed: Boolean = false
     
       // Whether the task has failed.
    -  @volatile private var failed: Boolean = false
    +  private var failed: Boolean = false
    +
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
     
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    -  override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +  @GuardedBy("this")
    +  override def addTaskCompletionListener(listener: TaskCompletionListener)
    +      : this.type = synchronized {
    +    if (completed) {
    +      listener.onTaskCompletion(this)
    --- End diff --
    
    Why would we do that, if we are going to rethrow the exception anyway? The only difference is that it would be a `TaskCompletionListenerException` instead. Calling `invokeListeners` would also call already invoked listeners, which is what we are trying to avoid.
    
    We are trying to void 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Ok, had a small discussion offline. It seems weird that we have different calling policies for failure and completion listeners. I am going to change the invocation of completion listeners to exactly once as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74335 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74335/testReport)** for PR 17244 at commit [`12f947e`](https://github.com/apache/spark/commit/12f947ea84c923ca546256249efea04abf189fde).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74323 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74323/testReport)** for PR 17244 at commit [`d16ad88`](https://github.com/apache/spark/commit/d16ad88d83351d8c514a389cf2b65605ee558b11).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Using `TaskContext. synchronized` out of `TaskContext` should not be encouraged. How about make `TaskContext .addTaskCompletionListener` check `isCompleted` internally? If it's done, don't add it to the list, then either ignore the listener or call the listener immediately.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r106318836
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -52,62 +63,79 @@ private[spark] class TaskContextImpl(
       @volatile private var interrupted: Boolean = false
     
       // Whether the task has completed.
    -  @volatile private var completed: Boolean = false
    +  private var completed: Boolean = false
     
       // Whether the task has failed.
    -  @volatile private var failed: Boolean = false
    +  private var failed: Boolean = false
    +
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
     
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    -  override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +  @GuardedBy("this")
    +  override def addTaskCompletionListener(listener: TaskCompletionListener)
    +      : this.type = synchronized {
    +    if (completed) {
    +      listener.onTaskCompletion(this)
    --- End diff --
    
    or call the `invokeListeners`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74404 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74404/testReport)** for PR 17244 at commit [`41448ab`](https://github.com/apache/spark/commit/41448abca620c7ee004f48669a1234e45c4805db).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74404 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74404/testReport)** for PR 17244 at commit [`41448ab`](https://github.com/apache/spark/commit/41448abca620c7ee004f48669a1234e45c4805db).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105489812
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    --- End diff --
    
    nit: method synchronized instead of block ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74405 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74405/testReport)** for PR 17244 at commit [`f3b9b97`](https://github.com/apache/spark/commit/f3b9b9798274343c14e8303ed2582f56eab9e31e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105490080
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    +      if (completed) {
    +        listener.onTaskCompletion(this)
    +      }
    +      // Always add the listener because it is legal to call them multiple times.
    --- End diff --
    
    I did not realize this, interesting !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r106577342
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -52,62 +63,79 @@ private[spark] class TaskContextImpl(
       @volatile private var interrupted: Boolean = false
     
       // Whether the task has completed.
    -  @volatile private var completed: Boolean = false
    +  private var completed: Boolean = false
     
       // Whether the task has failed.
    -  @volatile private var failed: Boolean = false
    +  private var failed: Boolean = false
    +
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
     
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    -  override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +  @GuardedBy("this")
    +  override def addTaskCompletionListener(listener: TaskCompletionListener)
    +      : this.type = synchronized {
    +    if (completed) {
    +      listener.onTaskCompletion(this)
    --- End diff --
    
    `invokeListeners` takes a list of listeners, so we are able to only call this listener.
    
    I think it's better to make these listeners consistent, i.e. throw `TaskCompletionListenerException` when failure happens during calling listener.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Thanks for the reviews! Merging to master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r106318801
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -52,62 +63,79 @@ private[spark] class TaskContextImpl(
       @volatile private var interrupted: Boolean = false
     
       // Whether the task has completed.
    -  @volatile private var completed: Boolean = false
    +  private var completed: Boolean = false
     
       // Whether the task has failed.
    -  @volatile private var failed: Boolean = false
    +  private var failed: Boolean = false
    +
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
     
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    -  override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +  @GuardedBy("this")
    +  override def addTaskCompletionListener(listener: TaskCompletionListener)
    +      : this.type = synchronized {
    +    if (completed) {
    +      listener.onTaskCompletion(this)
    --- End diff --
    
    shall we also try catch here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74323/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/17244


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74404/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105559900
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74327 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74327/testReport)** for PR 17244 at commit [`535349d`](https://github.com/apache/spark/commit/535349d91892754d7bad4da67c1d0419d1054022).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by sameeragarwal <gi...@git.apache.org>.
Github user sameeragarwal commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105779991
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContext.scala ---
    @@ -105,7 +105,11 @@ abstract class TaskContext extends Serializable {
     
       /**
        * Adds a (Java friendly) listener to be executed on task completion.
    -   * This will be called in all situation - success, failure, or cancellation.
    +   * This will be called in all situation - success, failure, or cancellation. Adding a listener
    +   * to an already completed task will result in that listeners being called immediately.
    --- End diff --
    
    micro nit: s/listeners/listener here and below


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74335 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74335/testReport)** for PR 17244 at commit [`12f947e`](https://github.com/apache/spark/commit/12f947ea84c923ca546256249efea04abf189fde).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105489927
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    +      if (completed) {
    +        listener.onTaskCompletion(this)
    +      }
    +      // Always add the listener because it is legal to call them multiple times.
    +      onCompleteCallbacks += listener
    +    }
         this
       }
     
    +  @GuardedBy("this")
       override def addTaskFailureListener(listener: TaskFailureListener): this.type = {
    -    onFailureCallbacks += listener
    +    synchronized {
    --- End diff --
    
    nit: method synchronized instead of block ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74466 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74466/testReport)** for PR 17244 at commit [`4199619`](https://github.com/apache/spark/commit/4199619f04dbd01c62fffe86b403f5fedc46c76e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105559906
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    +      if (completed) {
    +        listener.onTaskCompletion(this)
    +      }
    +      // Always add the listener because it is legal to call them multiple times.
    +      onCompleteCallbacks += listener
    +    }
         this
       }
     
    +  @GuardedBy("this")
       override def addTaskFailureListener(listener: TaskFailureListener): this.type = {
    -    onFailureCallbacks += listener
    +    synchronized {
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by sameeragarwal <gi...@git.apache.org>.
Github user sameeragarwal commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105780702
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContext.scala ---
    @@ -126,14 +134,14 @@ abstract class TaskContext extends Serializable {
       }
     
       /**
    -   * Adds a listener to be executed on task failure.
    -   * Operations defined here must be idempotent, as `onTaskFailure` can be called multiple times.
    --- End diff --
    
    Why delete this? `onTaskFailure` can also be called multiple times right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74405/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105788774
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContext.scala ---
    @@ -126,14 +134,14 @@ abstract class TaskContext extends Serializable {
       }
     
       /**
    -   * Adds a listener to be executed on task failure.
    -   * Operations defined here must be idempotent, as `onTaskFailure` can be called multiple times.
    --- End diff --
    
    This was disabled in https://github.com/apache/spark/pull/11504. So the comment does not make sense anymore.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105510888
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
    --- End diff --
    
    Yes, which is why I mentioned it as extension :-)
    For failed, it is already valid to remove volatile


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105491214
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
    --- End diff --
    
    If drop the volatility then we need to make `isCompleted` synchronized as well; to ensure safe publication.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105559913
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
     
    +  // Throwable that caused the task to fail
    +  private var failure: Throwable = _
    +
       // If there was a fetch failure in the task, we store it here, to make sure user-code doesn't
       // hide the exception.  See SPARK-19276
       @volatile private var _fetchFailedException: Option[FetchFailedException] = None
     
    +  @GuardedBy("this")
       override def addTaskCompletionListener(listener: TaskCompletionListener): this.type = {
    -    onCompleteCallbacks += listener
    +    synchronized {
    +      if (completed) {
    +        listener.onTaskCompletion(this)
    +      }
    +      // Always add the listener because it is legal to call them multiple times.
    --- End diff --
    
    I have updated the doc in `TaskContext` to reflect this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74335/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74466 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74466/testReport)** for PR 17244 at commit [`4199619`](https://github.com/apache/spark/commit/4199619f04dbd01c62fffe86b403f5fedc46c76e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105489232
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
    --- End diff --
    
    This need not be volatile anymore - given that it is updated and queried within a synchronized block.
    We could revisit for completed too - though that would be an extension.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by sameeragarwal <gi...@git.apache.org>.
Github user sameeragarwal commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105785854
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/TaskContextSuite.scala ---
    @@ -228,6 +228,31 @@ class TaskContextSuite extends SparkFunSuite with BeforeAndAfter with LocalSpark
         assert(res === Array("testPropValue,testPropValue"))
       }
     
    +  test("immediately call a completion listener if the context is completed") {
    +    var invocations = 0
    +    val context = TaskContext.empty()
    +    context.markTaskCompleted()
    +    context.addTaskCompletionListener(_ => invocations += 1)
    +    assert(invocations == 1)
    +    context.markTaskCompleted()
    +    assert(invocations == 2)
    --- End diff --
    
    can we call `context.markTaskCompleted()` once again and assert `invocations == 2` to have a test for idempotency?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74323 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74323/testReport)** for PR 17244 at commit [`d16ad88`](https://github.com/apache/spark/commit/d16ad88d83351d8c514a389cf2b65605ee558b11).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74405 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74405/testReport)** for PR 17244 at commit [`f3b9b97`](https://github.com/apache/spark/commit/f3b9b9798274343c14e8303ed2582f56eab9e31e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    **[Test build #74327 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/74327/testReport)** for PR 17244 at commit [`535349d`](https://github.com/apache/spark/commit/535349d91892754d7bad4da67c1d0419d1054022).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74327/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74466/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    cc @rxin @sameeragarwal @zsxwing 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #17244: [SPARK-19889][SQL] Make TaskContext callbacks thr...

Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/17244#discussion_r105559898
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskContextImpl.scala ---
    @@ -57,57 +68,75 @@ private[spark] class TaskContextImpl(
       // Whether the task has failed.
       @volatile private var failed: Boolean = false
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    LGTM. Would be great if other reviewers can also take a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #17244: [SPARK-19889][SQL] Make TaskContext callbacks thread saf...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/17244
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org