[SPARK-2492][Streaming] kafkaReceiver minor changes to align with Kafka 0.8 #1420

jerryshao · 2014-07-15T13:31:28Z

Update the KafkaReceiver's behavior when auto.offset.reset is set.

In Kafka 0.8, auto.offset.reset is a hint for out-range offset to seek to the beginning or end of the partition. While in the previous code auto.offset.reset is a enforcement to seek to the beginning or end immediately, this is different from Kafka 0.8 defined behavior.

Also deleting extesting ZK metadata in Receiver when multiple consumers are launched will introduce issue as mentioned in SPARK-2383.

So Here we change to offer user to API to explicitly reset offset before create Kafka stream, while in the meantime keep the same behavior as Kafka 0.8 for parameter auto.offset.reset.

@tdas, would you please review this PR? Thanks a lot.

SparkQA · 2014-07-15T13:37:52Z

QA tests have started for PR 1420. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16675/consoleFull

SparkQA · 2014-07-15T15:14:32Z

QA results for PR 1420:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16675/consoleFull

SparkQA · 2014-07-24T02:53:23Z

QA tests have started for PR 1420. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17088/consoleFull

SparkQA · 2014-07-24T03:35:43Z

QA results for PR 1420:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17088/consoleFull

SparkQA · 2014-07-24T07:28:36Z

QA tests have started for PR 1420. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17109/consoleFull

SparkQA · 2014-07-24T08:12:03Z

QA results for PR 1420:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17109/consoleFull

jerryshao · 2014-08-15T02:49:03Z

Hi @tdas , would you mind taking a look at this? thanks a lot.

mcnamaras · 2014-10-11T01:17:41Z

@tdas This would bring the kafka receiver in alignment with kafka 0.8. If folks would like the old kafka 0.7 behavior resetOffset does the trick. It looks like there are some merge conflicts right now, otherwise I would give this +1.

jerryshao · 2014-10-11T01:21:09Z

@reinvigorate Thanks for your review.

tdas · 2014-11-11T03:31:32Z

@jerryshao Could you merge conflicts? I am taking a look in trying to understand the change. After that I will merge this change.

tdas · 2014-11-11T03:59:18Z

external/kafka/src/main/scala/org/apache/spark/streaming/kafka/KafkaUtils.scala

+   * @param zkQuorum Zookeeper quorum (hostname:port,hostname:port,..).
+   * @param groupId The group id for this consumer.
+   */
+  def resetOffset(zkQuorum: String, groupId: String) {


I dont think it is good idea to expose have this utility function in our API that is very specific to a particular version of Kafka. If this functionality is required for some well-known version-specific Kafka issue, then I am guessing there will be known workarounds in the Kafka-world and we dont need to provide custom functionality for that. So I dont think we should add this.

So maybe we can just remove this function to let user to delete the ZK metadata by themselves if they know it.

…ZK metadata by calling this API

SparkQA · 2014-11-11T05:50:03Z

Test build #23199 has started for PR 1420 at commit 4a1c3f9.

This patch merges cleanly.

jerryshao · 2014-11-11T05:53:36Z

Hi TD, I've rebased the code to the latest master. I agree with you that providing this utility function for Kafka in Spark Streaming is quite strange, IMO we can just delete this code. If people want to directly read data from the begin or end of partition with such trick, they can do it by themselves. What is your opinion.

@reinvigorate , what is your opinion?

SparkQA · 2014-11-11T05:56:02Z

Test build #23199 has finished for PR 1420 at commit 4a1c3f9.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-11T05:56:03Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23199/
Test FAILed.

jerryshao · 2014-11-11T06:01:45Z

Jenkins, retest this please.

SparkQA · 2014-11-11T06:05:16Z

Test build #23201 has started for PR 1420 at commit 4a1c3f9.

This patch merges cleanly.

mcnamaras · 2014-11-11T06:09:14Z

what is your opinion?

@jerryshao Yeah I think @tdas is spot on.

SparkQA · 2014-11-11T06:11:48Z

Test build #23201 has finished for PR 1420 at commit 4a1c3f9.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-11T06:11:49Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23201/
Test FAILed.

jerryshao · 2014-11-11T06:15:18Z

Quite strange Jenkins fails to build this patch.

SparkQA · 2014-11-11T06:30:12Z

Test build #23203 has started for PR 1420 at commit de3a4c8.

This patch merges cleanly.

SparkQA · 2014-11-11T07:56:04Z

Test build #23203 has finished for PR 1420 at commit de3a4c8.

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

AmplabJenkins · 2014-11-11T07:56:07Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23203/
Test PASSed.

tdas · 2014-11-11T08:42:03Z

external/kafka/src/main/scala/org/apache/spark/streaming/kafka/KafkaUtils.scala

@@ -17,6 +17,7 @@

 package org.apache.spark.streaming.kafka

+


This is not necessary

tdas · 2014-11-11T08:43:30Z

Thank @reinvigorate. @jerryshao Could you please delete that method, and remove the ZKUtils import. Other than that this patch looks good to me.

SparkQA · 2014-11-11T09:19:59Z

Test build #23214 has started for PR 1420 at commit d6ae94d.

This patch merges cleanly.

jerryshao · 2014-11-11T09:26:38Z

Thanks @tdas and @reinvigorate , I've removed this method as you suggested, please help to review :)

tdas · 2014-11-11T09:29:11Z

LGTM. Will merge when tests pass.

tdas · 2014-11-11T10:16:30Z

KafkaStreamSuite has passed. So I am merging this.

SparkQA · 2014-11-11T10:43:07Z

Test build #23214 has finished for PR 1420 at commit d6ae94d.

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

AmplabJenkins · 2014-11-11T10:43:11Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23214/
Test PASSed.

@tdas

…ka 0.8 Update the KafkaReceiver's behavior when auto.offset.reset is set. In Kafka 0.8, `auto.offset.reset` is a hint for out-range offset to seek to the beginning or end of the partition. While in the previous code `auto.offset.reset` is a enforcement to seek to the beginning or end immediately, this is different from Kafka 0.8 defined behavior. Also deleting extesting ZK metadata in Receiver when multiple consumers are launched will introduce issue as mentioned in [SPARK-2383](https://issues.apache.org/jira/browse/SPARK-2383). So Here we change to offer user to API to explicitly reset offset before create Kafka stream, while in the meantime keep the same behavior as Kafka 0.8 for parameter `auto.offset.reset`. @tdas, would you please review this PR? Thanks a lot. Author: jerryshao <saisai.shao@intel.com> Closes #1420 from jerryshao/kafka-fix and squashes the following commits: d6ae94d [jerryshao] Address the comment to remove the resetOffset() function de3a4c8 [jerryshao] Fix compile error 4a1c3f9 [jerryshao] Doc changes b2c1430 [jerryshao] Move offset reset to a helper function to let user explicitly delete ZK metadata by calling this API fac8fd6 [jerryshao] Changes to align with Kafka 0.8 (cherry picked from commit c8850a3) Signed-off-by: Tathagata Das <tathagata.das1565@gmail.com>

jerryshao mentioned this pull request Nov 11, 2014

[SPARK-4062][Streaming]Add ReliableKafkaReceiver in Spark Streaming Kafka connector #2991

Closed

tdas reviewed Nov 11, 2014
View reviewed changes

jerryshao added 3 commits November 11, 2014 13:40

Changes to align with Kafka 0.8

fac8fd6

Move offset reset to a helper function to let user explicitly delete …

b2c1430

…ZK metadata by calling this API

Doc changes

4a1c3f9

jerryshao force-pushed the kafka-fix branch from b2eb397 to 4a1c3f9 Compare November 11, 2014 05:44

Fix compile error

de3a4c8

tdas reviewed Nov 11, 2014
View reviewed changes

Address the comment to remove the resetOffset() function

d6ae94d

asfgit closed this in c8850a3 Nov 11, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-2492][Streaming] kafkaReceiver minor changes to align with Kafka 0.8 #1420

[SPARK-2492][Streaming] kafkaReceiver minor changes to align with Kafka 0.8 #1420

jerryshao commented Jul 15, 2014

SparkQA commented Jul 15, 2014

SparkQA commented Jul 15, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

jerryshao commented Aug 15, 2014

mcnamaras commented Oct 11, 2014

jerryshao commented Oct 11, 2014

tdas commented Nov 11, 2014

tdas Nov 11, 2014

jerryshao Nov 11, 2014

SparkQA commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

mcnamaras commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

tdas Nov 11, 2014

tdas commented Nov 11, 2014

SparkQA commented Nov 11, 2014

jerryshao commented Nov 11, 2014

tdas commented Nov 11, 2014

tdas commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

[SPARK-2492][Streaming] kafkaReceiver minor changes to align with Kafka 0.8 #1420

[SPARK-2492][Streaming] kafkaReceiver minor changes to align with Kafka 0.8 #1420

Conversation

jerryshao commented Jul 15, 2014

SparkQA commented Jul 15, 2014

SparkQA commented Jul 15, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

SparkQA commented Jul 24, 2014

jerryshao commented Aug 15, 2014

mcnamaras commented Oct 11, 2014

jerryshao commented Oct 11, 2014

tdas commented Nov 11, 2014

tdas Nov 11, 2014

Choose a reason for hiding this comment

jerryshao Nov 11, 2014

Choose a reason for hiding this comment

SparkQA commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

mcnamaras commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

jerryshao commented Nov 11, 2014

SparkQA commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014

tdas Nov 11, 2014

Choose a reason for hiding this comment

tdas commented Nov 11, 2014

SparkQA commented Nov 11, 2014

jerryshao commented Nov 11, 2014

tdas commented Nov 11, 2014

tdas commented Nov 11, 2014

SparkQA commented Nov 11, 2014

AmplabJenkins commented Nov 11, 2014