Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#SPARK-2808 update kafka to version 0.8.2 #3631

Closed
wants to merge 3 commits into from

Conversation

helena
Copy link

@helena helena commented Dec 7, 2014

https://issues.apache.org/jira/browse/SPARK-2808 update kafka to version 0.8.2. Kafka 0.8.2 is in beta still.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@JoshRosen
Copy link
Contributor

Hi @helena,

Do you mind editing this PR's title to say "SPARK-2808" instead of #2808? This will ensure that this PR is properly linked to JIRA and our PR review dashboard.

Does this PR introduce any backwards-incompatible changes? If a user is using an older version of Kafka, will they have to update any code or configurations as a result of this patch?

@helena helena changed the title #2808 update kafka to version 0.8.2 #SPARK-2808 update kafka to version 0.8.2 Dec 22, 2014
@helena
Copy link
Author

helena commented Dec 22, 2014

@JoshRosen Ticket name updated :) Sorry for the delay, I was away.

@tdas
Copy link
Contributor

tdas commented Dec 24, 2014

Hey @helena I am not sure whether we want to upgrade to a beta version. Especially, there have been subtle changes within Kafka between releases and I am afraid to make changes (even more so with beta versions) without fully understanding implications. So could you elaborate on why you want to update to 0.8.2 beta ?

@helena
Copy link
Author

helena commented Dec 24, 2014

Hi @tdas, +1. This PR was done as initial push, and as soon as 0.8.2 is out of beta I planned to do the final commit, then have it considered. I never expected it to be merged while in beta.

To your other point, it is important to allow users to use the latest version of kafka in their applications which integrate with spark streaming kafka.

You can find a high-level description here http://blog.confluent.io/2014/12/02/whats-coming-in-apache-kafka-0-8-2 with full release notes here https://archive.apache.org/dist/kafka/0.8.2-beta/RELEASE_NOTES.html

@helena
Copy link
Author

helena commented Jan 8, 2015

Waiting for kafka 0.8.2 to move to GA, still beta

@rustyrazorblade
Copy link

Kafka 0.8.2 is now listed as latest stable.

@tdas
Copy link
Contributor

tdas commented Feb 4, 2015

Aah cool. However 0.8.1 and 0.8.2 have pretty big changes between them, so lets merge this for the next release. We are already doing a lot of experimental Kafka stuff in this release (feature merge window has closed).

@helena
Copy link
Author

helena commented Feb 4, 2015

@tdas what shall I do with this PR to complete it then?

@tdas
Copy link
Contributor

tdas commented Feb 4, 2015

The first step would be to see whether upgrading to 0.8.2 passes the
inbuilt unit-tests or not (including the new Kafka stuff).
In addition, is there are documentation somewhere that talks about the
compatibility of 0.8.2 with 0.8.0 / 0.8.1.1 kafka clusters? We can figure
out the path accordingly. In any case, it will be in the next release cycle
(1.4).

On Wed, Feb 4, 2015 at 6:02 AM, Helena Edelson notifications@github.com
wrote:

@tdas https://github.com/tdas what shall I do with this PR to complete
it then?


Reply to this email directly or view it on GitHub
#3631 (comment).

@helena
Copy link
Author

helena commented Feb 4, 2015

All streaming-kafka sbt tests and /dev/run-tests passed in my initial push as beta, and with the GA update all still pass. Should I resubmit this PR against another branch?

@tdas
Kafka version 0.8.2.0 will work with 0.8.0 / 0.8.1.1 clusters. https://cwiki.apache.org/confluence/display/KAFKA/A+Guide+To+The+Kafka+Protocol#AGuideToTheKafkaProtocol-VersioningandCompatibility however; features that you may want that are only in 0.8.2.0 (like not storing offsets in zookeeper but rather in kafka) should be turned on by default in your consumer (since it decides where to store the offsets). That "on" will make the consumer not work with an older version so you have to turn it off in that case and it should work. Little things like that.

@tdas
Copy link
Contributor

tdas commented Feb 5, 2015

Jenkins, this is ok to test.

@tdas
Copy link
Contributor

tdas commented Feb 5, 2015

Then for the new API (which does not store offsets externally) may need to be updated to ensure compatibility. I am not sure whether the current unit tests will catch such issues. This will require a bit of manual testing. Will do after 1.3 release rush. :)

Thanks for the link!

@SparkQA
Copy link

SparkQA commented Feb 5, 2015

Test build #26849 has started for PR 3631 at commit 2e67c66.

  • This patch does not merge cleanly.

@helena
Copy link
Author

helena commented Feb 5, 2015

NP, I think manual may not be necessary, just the addition of tests that automate testing the version or testing all and handling the execution path properly. I can add that when merged.

@SparkQA
Copy link

SparkQA commented Feb 5, 2015

Test build #26849 has finished for PR 3631 at commit 2e67c66.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

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

@srowen
Copy link
Member

srowen commented Feb 7, 2015

@helena I too would like to update to Kafka 0.8.2 as it does have a number of good improvements. can you rebase? @tdas are you OK with merging this for only master / 1.4.0?

@helena
Copy link
Author

helena commented Feb 7, 2015

Hi @srowen, will do.

@koeninger
Copy link
Contributor

This will need some changes to KafkaCluster and possibly other things related to the new api... let me know if you want a hand.

@helena
Copy link
Author

helena commented Feb 10, 2015

@koeninger Yes, there consumer offset and some other minor changes to make.
I'm finding it hard to carve out time for this, high ticket load this week. Go for it if you want, otherwise I'll do it later in the week.

@helena
Copy link
Author

helena commented Feb 11, 2015

@koeninger this is a definite blocker for me, I'm upgrading the connector to scala 2.11 with a cross build. Let me know if you have time, otherwise I will get back to this tomorrow morning.

@koeninger
Copy link
Contributor

@helena I updated it, pr is at #4537

@helena
Copy link
Author

helena commented Feb 11, 2015

Great I'll close this.

@helena helena closed this Feb 11, 2015
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.

8 participants