Skip to content

Conversation

tdas
Copy link
Contributor

@tdas tdas commented Apr 23, 2018

What changes were proposed in this pull request?

Currently, the driver side of the Kafka source (i.e. KafkaMicroBatchReader) eagerly creates a consumer as soon as the Kafk aMicroBatchReader is created. However, we create dummy KafkaMicroBatchReader to get the schema and immediately stop it. Its better to make the consumer creation lazy, it will be created on the first attempt to fetch offsets using the KafkaOffsetReader.

How was this patch tested?

Existing unit tests

@tdas
Copy link
Contributor Author

tdas commented Apr 23, 2018

@brkyvz

@tdas tdas changed the title [SPARK-24056] Make consumer creation lazy [SPARK-24056] Make Kafka consumer creation lazy Apr 23, 2018
@tdas tdas changed the title [SPARK-24056] Make Kafka consumer creation lazy [SPARK-24056] [SS] Make consumer creation lazy in Kafka source for Structured streaming Apr 23, 2018
@SparkQA
Copy link

SparkQA commented Apr 23, 2018

Test build #89744 has finished for PR 21134 at commit 7246b9b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 24, 2018

Test build #89750 has finished for PR 21134 at commit a0f3977.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 24, 2018

Test build #89771 has finished for PR 21134 at commit 77e2942.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

@brkyvz brkyvz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

val kafkaReaderThread = Executors.newSingleThreadExecutor(new ThreadFactory {
override def newThread(r: Runnable): Thread = {
val t = new UninterruptibleThread("Kafka Offset Reader") {
val t = new UninterruptibleThread(s"Kafka Offset Reader") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uber nit: don't need s

@SparkQA
Copy link

SparkQA commented Apr 24, 2018

Test build #89801 has finished for PR 21134 at commit 858596e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@asfgit asfgit closed this in 7b1e652 Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants