KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0 by mjsax · Pull Request #4779 · apache/kafka

mjsax · 2018-03-27T03:49:48Z

No description provided.

mjsax · 2018-03-27T03:54:04Z

This is the port of #4746 (0.10.1), #4758 (0.10.2), #4761 (0.11.0), #4768 (1.0), and #4773 (1.11) to trunk`.

Note: 1.0 was the first branch that did contain upgrade system tests already -- I just added the new once -- we can consider to consolidate them in this PR already. LMKWYT.

If we decide to cleanup tests in 1.0/1.1, those changes need to be added to this branch, too. System tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1605/

Note: we cannot add upgrade tests for 1.1 yet, as 1.1 is not released yet. I have a branch ready and will open PR after 1.1 is released to add those missing tests.

This PR does not implement KIP-268 completely, but only port parameter upgrade.from -- for new metadata version 3 and "version probing" there will be a separate PR (#4636).

\cc @bbejeck @vvcephei

guozhangwang · 2018-03-27T23:13:52Z

retest this please

guozhangwang · 2018-03-28T04:26:30Z

Just for my own education: why we want to only include example jars if it is not for upgrade tests?

We don't need them for upgrade test -- it's just to reduce CLASSPATH to the minimum for this case.

guozhangwang · 2018-03-28T04:28:14Z

Hmm.. seems this cherry-picking not correct, as it is for trunk the upgrade section should include for 1.2.0 right?

guozhangwang · 2018-03-28T04:29:45Z

The title should still be Upgrading a 1.0.0 Kafka Streams Application?

Actually I think the title was not consistent: previous the title of Upgrading a <XXX> version talks about how to upgrade from 's previous minor release to where here we are talking about upgrading from to 1.0.0.

How about rename the title to Upgrading to a 1.0.0 Kafka Streams Application, and use a sebsuction
for Upgrading from 0.11.0, and so on for the following sub sections.

Yes, the "id" means upgrade to this version while the text means upgrade from this version. It's explained in the first bullet point below thought. I would prefer to do a separate PR if we want to rephrase -- this PR is already complex enough.

guozhangwang · 2018-03-28T04:38:06Z

Ditto as above.

guozhangwang · 2018-03-28T04:38:23Z

ditto as above.

guozhangwang · 2018-03-28T04:47:50Z

Do we need to ignore this test as well?

bbejeck

Overall looks good, just a couple of minor comments/questions.

bbejeck · 2018-03-30T14:51:40Z

should this be to 1.2.0 ?

bbejeck · 2018-03-30T15:02:25Z

references here to version 1.1.1 should these be 1.2.0?

bbejeck · 2018-03-30T15:23:44Z

This version of StreamsUpgradeTest doesn't have the upgraded from print statement, was that intentional? The other version have it.

This is intentional, because we now pass in upgrade.from via the propertiesFile and the prop file gets printed.

vvcephei

Just a couple of minor comments.

vvcephei · 2018-03-30T16:39:43Z

Do we need "notable changes" for 1.2 as well?

vvcephei · 2018-03-30T16:45:29Z

I guess we can add 1.1 now!

vvcephei · 2018-03-30T16:46:20Z

we can also add 1.1 here.

I actually think, this is the root cause of the issue... Scala 2.12 required JDK8...

We cannot. Need to downgrade to 2.11 anyway. Also, we can only pick one Scala version as there is only one directory for the Kafka version. We would need to change a lot of stuff to encode the Scala version in the directory(structure) to support different version. Even if we want to do this, it would be a separate PR.

I didn't quite follow this. If you're saying that switching to scala 2.12 caused the runtime error about class version, that sounds about right.

But does this mean that we cannot add Kafka 1.1.x?

Sorry for the confusion -- my bad. We an add 1.1.x -- will update this PR after I merged the 1.1 PR and cherry-picked those latest changes.

Update upgrade docs Github comments John's review Guozhang's follow up

remove generics move config fix doc typos remove generics move config Consolidating system tests Addressed review comment Update test-dev version to 1.1.1-SNAPSHOT Change Scala Version Add comments to vagrant/base.sh and update Scala versions for Dockerfile

mjsax · 2018-04-06T05:28:58Z

Updated this PR and triggered system tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1660/

Note, this does only port the 1.1 branch PR but does not yet add new tests on purpose. Upgrading the metadata to version 3 will be a separate PR after this one is merged.

dguy

Just one super nit, otherwise LGTM

dguy · 2018-04-06T14:12:02Z

+public class StreamsUpgradeTest {
+
+    /**
+     * This test cannot be run executed, as long as Kafka 0.10.1.2 is not released


nit: "cannot be run" or "cannot be executed" same in other versions of this test

mjsax · 2018-04-06T20:43:14Z

Rebasing broke the system tests. Pushed some fixed. Retriggered: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1664/ (passed)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Bill Bejeck <bill@confluent.io>, John Roesler <john@confluent.io>, Damian Guy <damian@confluent.io>

mjsax added the streams label Mar 27, 2018

mjsax requested review from dguy and guozhangwang March 27, 2018 03:49

mjsax force-pushed the kafka-6054-fix-upgrade-trunk branch from f13fc13 to 41db9c6 Compare March 27, 2018 04:37

guozhangwang reviewed Mar 28, 2018

View reviewed changes

bbejeck reviewed Mar 30, 2018

View reviewed changes

vvcephei approved these changes Mar 30, 2018

View reviewed changes

mjsax force-pushed the kafka-6054-fix-upgrade-trunk branch from cf05a96 to 4d8c7f6 Compare March 30, 2018 18:58

mjsax added 3 commits April 5, 2018 17:30

KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0

610a5b7

Update upgrade docs Github comments John's review Guozhang's follow up

fix doc typos

3758b15

remove generics move config fix doc typos remove generics move config Consolidating system tests Addressed review comment Update test-dev version to 1.1.1-SNAPSHOT Change Scala Version Add comments to vagrant/base.sh and update Scala versions for Dockerfile

Review comments

8e89c7a

mjsax force-pushed the kafka-6054-fix-upgrade-trunk branch from 4d8c7f6 to 8e89c7a Compare April 6, 2018 05:22

dguy approved these changes Apr 6, 2018

View reviewed changes

bbejeck approved these changes Apr 6, 2018

View reviewed changes

mjsax added 2 commits April 6, 2018 13:39

fix system tests

2e0920e

Github comment

5d3ab01

mjsax merged commit 0c0d836 into apache:trunk Apr 7, 2018

mjsax mentioned this pull request Apr 20, 2018

KAFKA-6054: Add 'version probing' to Kafka Streams rebalance #4636

Merged

Conversation

mjsax commented Mar 27, 2018

Uh oh!

mjsax commented Mar 27, 2018

Uh oh!

guozhangwang commented Mar 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

for Upgrading from 0.11.0, and so on for the following sub sections.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbejeck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vvcephei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjsax commented Apr 6, 2018

Uh oh!

dguy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjsax commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

for `Upgrading from 0.11.0`, and so on for the following sub sections.

mjsax commented Apr 6, 2018 •

edited

Loading