You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dilipbiswal <gi...@git.apache.org> on 2018/10/08 09:11:46 UTC

[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

GitHub user dilipbiswal opened a pull request:

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

    [SPARK-25615][SQL][TEST] Improve the test runtime of KafkaSinkSuite: streaming write to non-existing topic

    ## What changes were proposed in this pull request?
    Specify `kafka.max.block.ms` to 10 seconds while creating the kafka writer. In the absence of this overridden config, by default it uses a default time out of 60 seconds.
    
    With this change the test completes in close to 10 seconds as opposed to 1 minute.
    
    ## How was this patch tested?
    This is a test fix.

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

    $ git pull https://github.com/dilipbiswal/spark SPARK-25615

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

    https://github.com/apache/spark/pull/22671.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 #22671
    
----
commit 6a36250dec0e23520f482d8d092cb658ce147fb7
Author: Dilip Biswal <db...@...>
Date:   2018-10-08T09:07:47Z

    [SPARK-25615] Improve the test runtime of KafkaSinkSuite: streaming - write to non-existing topic

----


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    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 #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    cc @gatorsmile 


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

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


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #97108 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97108/testReport)** for PR 22671 at commit [`6a36250`](https://github.com/apache/spark/commit/6a36250dec0e23520f482d8d092cb658ce147fb7).
     * 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 #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #97108 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97108/testReport)** for PR 22671 at commit [`6a36250`](https://github.com/apache/spark/commit/6a36250dec0e23520f482d8d092cb658ce147fb7).


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #97170 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97170/testReport)** for PR 22671 at commit [`16b23a8`](https://github.com/apache/spark/commit/16b23a84719ec19bf03194e27d35edf4655fe85a).
     * 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 pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

    https://github.com/apache/spark/pull/22671#discussion_r223775935
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSinkSuite.scala ---
    @@ -332,7 +332,9 @@ class KafkaSinkSuite extends StreamTest with SharedSQLContext with KafkaTest {
         var ex: Exception = null
         try {
           ex = intercept[StreamingQueryException] {
    -        writer = createKafkaWriter(input.toDF(), withTopic = Some(topic))()
    +        writer = createKafkaWriter(input.toDF(),
    --- End diff --
    
    In retrospect 1000 may have been too low - that could cause flakiness if the test runner is slow or something and it legitimately takes a second or two for Kafka to respond. 10k definitely should work, and I'd be fine with 1k if we're really worried about test time here.


---

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


[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

    https://github.com/apache/spark/pull/22671#discussion_r223771001
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSinkSuite.scala ---
    @@ -332,7 +332,9 @@ class KafkaSinkSuite extends StreamTest with SharedSQLContext with KafkaTest {
         var ex: Exception = null
         try {
           ex = intercept[StreamingQueryException] {
    -        writer = createKafkaWriter(input.toDF(), withTopic = Some(topic))()
    +        writer = createKafkaWriter(input.toDF(),
    --- End diff --
    
    @srowen Thank you. I thought about it initially, but went with a conservative fix.
    
    > it seems like it's a generally good idea to not let anything block that long in tests
    I agree.
    
    > In fact KafkaContinuousSinkSuite sets it to 1000. We should standardize that too
    Yeah.. wasn't sure on value to set. If we think 1 second is a good enough timeout setting, i can change.


---

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


[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

    https://github.com/apache/spark/pull/22671#discussion_r223776123
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSinkSuite.scala ---
    @@ -332,7 +332,9 @@ class KafkaSinkSuite extends StreamTest with SharedSQLContext with KafkaTest {
         var ex: Exception = null
         try {
           ex = intercept[StreamingQueryException] {
    -        writer = createKafkaWriter(input.toDF(), withTopic = Some(topic))()
    +        writer = createKafkaWriter(input.toDF(),
    --- End diff --
    
    Honestly I think it's worth trying. Many Kafka tests are slow, and this could speed up many of them. What happens if it's 1 second everywhere? or even just 10?


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    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/3831/
    Test PASSed.


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    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/3791/
    Test PASSed.


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #97170 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97170/testReport)** for PR 22671 at commit [`16b23a8`](https://github.com/apache/spark/commit/16b23a84719ec19bf03194e27d35edf4655fe85a).


---

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


[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

    https://github.com/apache/spark/pull/22671#discussion_r223819177
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSinkSuite.scala ---
    @@ -332,7 +332,9 @@ class KafkaSinkSuite extends StreamTest with SharedSQLContext with KafkaTest {
         var ex: Exception = null
         try {
           ex = intercept[StreamingQueryException] {
    -        writer = createKafkaWriter(input.toDF(), withTopic = Some(topic))()
    +        writer = createKafkaWriter(input.toDF(),
    --- End diff --
    
    @srowen I have used 5 sec timeout for now and i have moved it to createKafkaWriter per ur suggestion. Pl.. let me know if you are okay. Currently there is only one test that make uses of this timeout value. So keep it conservatively at 5 secs ?


---

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


[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

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


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #4367 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4367/testReport)** for PR 22671 at commit [`16b23a8`](https://github.com/apache/spark/commit/16b23a84719ec19bf03194e27d35edf4655fe85a).
     * 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 #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    Merged to master


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    **[Test build #4367 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4367/testReport)** for PR 22671 at commit [`16b23a8`](https://github.com/apache/spark/commit/16b23a84719ec19bf03194e27d35edf4655fe85a).


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    I like the change. This test is down to 5 seconds now. Unfortunately I don't see speedup in other Kafka tests, but I think we should leave this in.


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    cc @zsxwing @tdas @jose-torres 


---

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


[GitHub] spark pull request #22671: [SPARK-25615][SQL][TEST] Improve the test runtime...

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

    https://github.com/apache/spark/pull/22671#discussion_r223752722
  
    --- Diff: external/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaSinkSuite.scala ---
    @@ -332,7 +332,9 @@ class KafkaSinkSuite extends StreamTest with SharedSQLContext with KafkaTest {
         var ex: Exception = null
         try {
           ex = intercept[StreamingQueryException] {
    -        writer = createKafkaWriter(input.toDF(), withTopic = Some(topic))()
    +        writer = createKafkaWriter(input.toDF(),
    --- End diff --
    
    Can we set this inside `createKafkaWriter`? it seems like it's a generally good idea to not let anything block that long in tests. In fact KafkaContinuousSinkSuite sets it to 1000. We should standardize that too.


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    Thanks a LOT @srowen 


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    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 #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

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


---

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


[GitHub] spark issue #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

    https://github.com/apache/spark/pull/22671
  
    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 #22671: [SPARK-25615][SQL][TEST] Improve the test runtime of Kaf...

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

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


---

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