You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sarutak <gi...@git.apache.org> on 2015/04/08 11:00:13 UTC

[GitHub] spark pull request: [SPARK-6769] Usage of the ListenerBus in YarnC...

GitHub user sarutak opened a pull request:

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

    [SPARK-6769] Usage of the ListenerBus in YarnClusterSuite is wrong

    In YarnClusterSuite, a test case uses `SaveExecutorInfo`  to handle ExecutorAddedEvent as follows.
    
    ```
    private class SaveExecutorInfo extends SparkListener {
      val addedExecutorInfos = mutable.Map[String, ExecutorInfo]()
    
      override def onExecutorAdded(executor: SparkListenerExecutorAdded) {
        addedExecutorInfos(executor.executorId) = executor.executorInfo
      }
    }
    
    ...
    
        listener = new SaveExecutorInfo
        val sc = new SparkContext(new SparkConf()
          .setAppName("yarn \"test app\" 'with quotes' and \\back\\slashes and $dollarSigns"))
        sc.addSparkListener(listener)
        val status = new File(args(0))
        var result = "failure"
        try {
          val data = sc.parallelize(1 to 4, 4).collect().toSet
          assert(sc.listenerBus.waitUntilEmpty(WAIT_TIMEOUT_MILLIS))
          data should be (Set(1, 2, 3, 4))
          result = "success"
        } finally {
          sc.stop()
          Files.write(result, status, UTF_8)
        }
    ```
    
    But, the usage is wrong because Executors will spawn during initializing SparkContext and SparkContext#addSparkListener should be invoked after the initialization, thus after Executors spawn, so SaveExecutorInfo cannot handle ExecutorAddedEvent.
    
    Following code refers the result of the handling ExecutorAddedEvent. Because of the reason above, we cannot reach the assertion. 
    
    ```
        // verify log urls are present
        listener.addedExecutorInfos.values.foreach { info =>
          assert(info.logUrlMap.nonEmpty)
        }
    ```

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

    $ git pull https://github.com/sarutak/spark SPARK-6769

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

    https://github.com/apache/spark/pull/5417.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 #5417
    
----
commit 153a91bf1c6365abd33c7a2dd0112c503790f616
Author: Kousuke Saruta <sa...@oss.nttdata.co.jp>
Date:   2015-04-08T08:58:50Z

    Fixed the usage of listener bus in YarnClusterSuite

----


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92781155
  
      [Test build #30243 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30243/consoleFull) for   PR 5417 at commit [`591cf3e`](https://github.com/apache/spark/commit/591cf3e79e1dfc492a2ca110b42606e84fac98ca).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92782534
  
      [Test build #30244 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30244/consoleFull) for   PR 5417 at commit [`e258530`](https://github.com/apache/spark/commit/e258530b80bedc6d0cfa678ecf0b53929c939dbe).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28297135
  
    --- Diff: yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala ---
    @@ -323,7 +321,15 @@ private object YarnClusterDriver extends Logging with Matchers {
         }
     
         // verify log urls are present
    -    listener.addedExecutorInfos.values.foreach { info =>
    +    val listenerOpt = sc.listenerBus.listeners.find {
    --- End diff --
    
    Yes, finder method in `ListenerBus` is good idea. 


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28348092
  
    --- Diff: core/src/main/scala/org/apache/spark/util/ListenerBus.scala ---
    @@ -19,6 +19,10 @@ package org.apache.spark.util
     
     import java.util.concurrent.CopyOnWriteArrayList
     
    +import org.apache.spark.scheduler.SparkListener
    --- End diff --
    
    nit: import order


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28314914
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala ---
    @@ -65,16 +66,22 @@ class LogUrlsStandaloneSuite extends FunSuite with LocalSparkContext {
             new MySparkConf().setAll(getAll)
           }
         }
    -    val conf = new MySparkConf()
    +    val conf = new MySparkConf().set(
    +      "spark.extraListeners",
    +      "org.apache.spark.deploy.SaveExecutorInfo")
    --- End diff --
    
    One line if you can? to establish this is a key-value pair?


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28165146
  
    --- Diff: yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala ---
    @@ -306,10 +305,9 @@ private object YarnClusterDriver extends Logging with Matchers {
           System.exit(1)
         }
     
    -    listener = new SaveExecutorInfo
         val sc = new SparkContext(new SparkConf()
    +      .set("spark.extraListeners", "org.apache.spark.deploy.yarn.SaveExecutorInfo")
    --- End diff --
    
    Man I really dislike this API, but it's what we have now...
    
    nit: `classOf[SaveExecutorInfo].getName()` instead of the hardcoded string.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92596601
  
      [Test build #30219 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30219/consoleFull) for   PR 5417 at commit [`207d325`](https://github.com/apache/spark/commit/207d325528c364c39fc67ed52826e5d9ea37e40c).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92780640
  
      [Test build #30243 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30243/consoleFull) for   PR 5417 at commit [`591cf3e`](https://github.com/apache/spark/commit/591cf3e79e1dfc492a2ca110b42606e84fac98ca).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28314898
  
    --- Diff: core/src/main/scala/org/apache/spark/util/ListenerBus.scala ---
    @@ -64,4 +68,12 @@ private[spark] trait ListenerBus[L <: AnyRef, E] extends Logging {
        */
       def onPostEvent(listener: L, event: E): Unit
     
    +  private[spark] def findListenersByClass[T <: L : ClassTag](): Seq[T] = {
    +    val c = implicitly[ClassTag[T]].runtimeClass
    +    listeners.filter {
    --- End diff --
    
    Isn't this a lot simpler as `.filter(_.getClass == c)`?


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90883350
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29854/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90873721
  
      [Test build #29852 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29852/consoleFull) for   PR 5417 at commit [`153a91b`](https://github.com/apache/spark/commit/153a91bf1c6365abd33c7a2dd0112c503790f616).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class SlowTestReceiver(totalRecords: Int, recordsPerSecond: Int)`
    
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90864889
  
      [Test build #29854 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29854/consoleFull) for   PR 5417 at commit [`3874adf`](https://github.com/apache/spark/commit/3874adfd2efbd934e4410caaf31060423453bc92).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92782535
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30244/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-91419960
  
      [Test build #30000 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30000/consoleFull) for   PR 5417 at commit [`2d7e409`](https://github.com/apache/spark/commit/2d7e4094f8d3c1f937ecf5dfa119483daceea2e6).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92788585
  
      [Test build #30245 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30245/consoleFull) for   PR 5417 at commit [`8adc8ba`](https://github.com/apache/spark/commit/8adc8ba412e9a02af6750b052a0c9acb81c442a0).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90854029
  
      [Test build #29852 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29852/consoleFull) for   PR 5417 at commit [`153a91b`](https://github.com/apache/spark/commit/153a91bf1c6365abd33c7a2dd0112c503790f616).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28370055
  
    --- Diff: yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala ---
    @@ -323,7 +321,12 @@ private object YarnClusterDriver extends Logging with Matchers {
         }
     
         // verify log urls are present
    -    listener.addedExecutorInfos.values.foreach { info =>
    +    val listeners = sc.listenerBus.findListenersByClass[SaveExecutorInfo]
    +    assert(listeners.size == 1)
    --- End diff --
    
    same 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 pull request: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28314930
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala ---
    @@ -65,16 +66,22 @@ class LogUrlsStandaloneSuite extends FunSuite with LocalSparkContext {
             new MySparkConf().setAll(getAll)
           }
         }
    -    val conf = new MySparkConf()
    +    val conf = new MySparkConf().set(
    +      "spark.extraListeners",
    +      "org.apache.spark.deploy.SaveExecutorInfo")
         sc = new SparkContext("local-cluster[2,1,512]", "test", conf)
     
    -    val listener = new SaveExecutorInfo
    -    sc.addSparkListener(listener)
    -
         // Trigger a job so that executors get added
         sc.parallelize(1 to 100, 4).map(_.toString).count()
     
         assert(sc.listenerBus.waitUntilEmpty(WAIT_TIMEOUT_MILLIS))
    +    val listenerOpt = sc.listenerBus.listeners.find {
    --- End diff --
    
    Can this use the new find method above or did I miss something?


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92782040
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30242/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28319172
  
    --- Diff: core/src/main/scala/org/apache/spark/util/ListenerBus.scala ---
    @@ -64,4 +68,12 @@ private[spark] trait ListenerBus[L <: AnyRef, E] extends Logging {
        */
       def onPostEvent(listener: L, event: E): Unit
     
    +  private[spark] def findListenersByClass[T <: L : ClassTag](): Seq[T] = {
    +    val c = implicitly[ClassTag[T]].runtimeClass
    +    listeners.filter {
    --- End diff --
    
    Oh... how embarrassing... Thanks.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92638961
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30219/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92638878
  
      [Test build #30219 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30219/consoleFull) for   PR 5417 at commit [`207d325`](https://github.com/apache/spark/commit/207d325528c364c39fc67ed52826e5d9ea37e40c).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `case class MinOf(left: Expression, right: Expression) extends Expression `
    
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28165225
  
    --- Diff: yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala ---
    @@ -323,7 +321,15 @@ private object YarnClusterDriver extends Logging with Matchers {
         }
     
         // verify log urls are present
    -    listener.addedExecutorInfos.values.foreach { info =>
    +    val listenerOpt = sc.listenerBus.listeners.find {
    --- End diff --
    
    You could place this code somewhere more common, like a companion object for `SaveExecutorInfo` or even a `private[spark]` method in `ListenerBus`. What do you think?


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90883333
  
      [Test build #29854 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/29854/consoleFull) for   PR 5417 at commit [`3874adf`](https://github.com/apache/spark/commit/3874adfd2efbd934e4410caaf31060423453bc92).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92853720
  
      [Test build #30245 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30245/consoleFull) for   PR 5417 at commit [`8adc8ba`](https://github.com/apache/spark/commit/8adc8ba412e9a02af6750b052a0c9acb81c442a0).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5417#issuecomment-93058457
  
    I'm merging this into master after fixing the things I pointed out myself. Thanks @sarutak @vanzin


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/5417#issuecomment-92985741
  
    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 pull request: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-90873733
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29852/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-91432980
  
      [Test build #30000 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30000/consoleFull) for   PR 5417 at commit [`2d7e409`](https://github.com/apache/spark/commit/2d7e4094f8d3c1f937ecf5dfa119483daceea2e6).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28369837
  
    --- Diff: core/src/test/scala/org/apache/spark/deploy/LogUrlsStandaloneSuite.scala ---
    @@ -65,16 +66,17 @@ class LogUrlsStandaloneSuite extends FunSuite with LocalSparkContext {
             new MySparkConf().setAll(getAll)
           }
         }
    -    val conf = new MySparkConf()
    +    val conf = new MySparkConf().set(
    +      "spark.extraListeners", classOf[SaveExecutorInfo].getName)
         sc = new SparkContext("local-cluster[2,1,512]", "test", conf)
     
    -    val listener = new SaveExecutorInfo
    -    sc.addSparkListener(listener)
    -
         // Trigger a job so that executors get added
         sc.parallelize(1 to 100, 4).map(_.toString).count()
     
         assert(sc.listenerBus.waitUntilEmpty(WAIT_TIMEOUT_MILLIS))
    +    val listeners = sc.listenerBus.findListenersByClass[SaveExecutorInfo]
    +    assert(listeners.size == 1)
    --- End diff --
    
    should be `===`


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-91433028
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30000/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

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


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92781159
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30243/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92782208
  
      [Test build #30244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30244/consoleFull) for   PR 5417 at commit [`e258530`](https://github.com/apache/spark/commit/e258530b80bedc6d0cfa678ecf0b53929c939dbe).


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:

    https://github.com/apache/spark/pull/5417#issuecomment-91534439
  
    CC @vanzin


---
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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#issuecomment-92853807
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30245/
    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: [SPARK-6769][YARN][TEST] Usage of the Listener...

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

    https://github.com/apache/spark/pull/5417#discussion_r28165362
  
    --- Diff: yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala ---
    @@ -323,7 +321,15 @@ private object YarnClusterDriver extends Logging with Matchers {
         }
     
         // verify log urls are present
    -    listener.addedExecutorInfos.values.foreach { info =>
    +    val listenerOpt = sc.listenerBus.listeners.find {
    +      case _: SaveExecutorInfo => true
    +      case _ => false
    +    }.map(_.asInstanceOf[SaveExecutorInfo])
    +    assert(listenerOpt.isDefined)
    --- End diff --
    
    The assert is unnecessary since `.get` will throw if it's not defined.


---
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