You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ifilonenko <gi...@git.apache.org> on 2018/09/13 20:28:44 UTC

[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

GitHub user ifilonenko opened a pull request:

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

    [SPARK-25291][K8S] Fixing Flakiness of Executor Pod tests

    ## What changes were proposed in this pull request?
    
    Added fix to flakiness that was present in PySpark tests w.r.t Executors not being tested. 
    
    Important fix to executorConf which was failing tests when executors *were* tested
    
    ## How was this patch tested?
    
    Unit and Integration tests


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

    $ git pull https://github.com/ifilonenko/spark SPARK-25291

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

    https://github.com/apache/spark/pull/22415.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 #22415
    
----
commit 33bf91eefc42fd09fa977c655a776c05733e1e94
Author: Ilan Filonenko <if...@...>
Date:   2018-09-13T20:22:48Z

    fixing flakiness by adding watchers to executor pods

----


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3164/



---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217583729
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    --- End diff --
    
    Why you chose to use a watch instead of listing?


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    @holdenk @felixcheung @liyinan926 further thoughts before merging?


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217609720
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    --- End diff --
    
    To make sure that an executor pod has been properly brought up, so we can analyze its logs. 


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    **[Test build #96053 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96053/testReport)** for PR 22415 at commit [`1c66537`](https://github.com/apache/spark/commit/1c66537b9c514dfd73a86c967478f4a6067ac332).


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3096/



---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    @mccheah for merge


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    @holdenk @liyinan926 @felixcheung for merge since @mccheah is out 


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    **[Test build #96145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96145/testReport)** for PR 22415 at commit [`9b00bc4`](https://github.com/apache/spark/commit/9b00bc4b87020f36504039ba2f8c50d6035f99c1).


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    **[Test build #96050 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96050/testReport)** for PR 22415 at commit [`33bf91e`](https://github.com/apache/spark/commit/33bf91eefc42fd09fa977c655a776c05733e1e94).


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3098/



---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test starting
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3096/



---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3164/



---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3098/
    Test PASSed.


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3164/
    Test PASSed.


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    I'm gonna merge this to master and branch-2.4 before noon if there's no more comment.


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217543984
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    So this only checks the executor pod at the top of the stack?


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217561045
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    Well they’d all be identical. If it is necessary we can do a .foreach{} but I think it might be extraneous, no? 


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    **[Test build #96050 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96050/testReport)** for PR 22415 at commit [`33bf91e`](https://github.com/apache/spark/commit/33bf91eefc42fd09fa977c655a776c05733e1e94).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Kubernetes integration test status success
    URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/3098/



---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

    https://github.com/apache/spark/pull/22415
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3096/
    Test PASSed.


---

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


[GitHub] spark issue #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor Pod test...

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

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


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217769122
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    While, it's not necessary for now, but it might be in the future. So I would suggest keeping the `foreach`.


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217560548
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    Why? 


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217560220
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    Yeah, no need to check other executors. 


---

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


[GitHub] spark pull request #22415: [SPARK-25291][K8S] Fixing Flakiness of Executor P...

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

    https://github.com/apache/spark/pull/22415#discussion_r217769238
  
    --- Diff: resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala ---
    @@ -218,17 +223,25 @@ private[spark] class KubernetesSuite extends SparkFunSuite
           .getItems
           .get(0)
         driverPodChecker(driverPod)
    -
    -    val executorPods = kubernetesTestComponents.kubernetesClient
    +    val execPods = scala.collection.mutable.Stack[Pod]()
    +    val execWatcher = kubernetesTestComponents.kubernetesClient
           .pods()
           .withLabel("spark-app-locator", appLocator)
           .withLabel("spark-role", "executor")
    -      .list()
    -      .getItems
    -    executorPods.asScala.foreach { pod =>
    -      executorPodChecker(pod)
    -    }
    -
    +      .watch(new Watcher[Pod] {
    +        logInfo("Beginning watch of executors")
    +        override def onClose(cause: KubernetesClientException): Unit =
    +          logInfo("Ending watch of executors")
    +        override def eventReceived(action: Watcher.Action, resource: Pod): Unit = {
    +          action match {
    +            case Action.ADDED | Action.MODIFIED =>
    +              execPods.push(resource)
    +          }
    +        }
    +      })
    +    Eventually.eventually(TIMEOUT, INTERVAL) { execPods.nonEmpty should be (true) }
    +    execWatcher.close()
    +    executorPodChecker(execPods.pop())
    --- End diff --
    
    I mean that's what the old behaviour did. If it's going to add a lot of delay to check the different pods then yeah lets simplify it, but it could make sense to check all of them since we do expect them to be the same and if we don something which breaks that assumption it would be good to know.


---

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