[SPARK-25615][SQL][TEST] Improve the test runtime of KafkaSinkSuite: streaming write to non-existing topic #22671
Conversation
… write to non-existing topic
|
Test build #97108 has finished for PR 22671 at commit
|
|
cc @gatorsmile |
| @@ -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(), | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@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 ? FYI - i tried using a 1 sec timeout and this suite passes on my laptop.
|
Test build #97170 has finished for PR 22671 at commit
|
|
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. |
|
Test build #4367 has finished for PR 22671 at commit
|
|
Merged to master |
|
Thanks a LOT @srowen |
…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. Closes apache#22671 from dilipbiswal/SPARK-25615. Authored-by: Dilip Biswal <dbiswal@us.ibm.com> Signed-off-by: Sean Owen <sean.owen@databricks.com>
What changes were proposed in this pull request?
Specify
kafka.max.block.msto 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.