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

mjsax · 2018-03-23T23:48:05Z

No description provided.

mjsax · 2018-03-23T23:50:33Z

This is the port of #4746 (0.10.1), #4758 (0.10.2), and #4761 (0.11.0) to branch 1.0.

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.

\cc @bbejeck @vvcephei

mjsax · 2018-03-25T21:09:27Z

Upgrade system tests passed: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1598/

Triggered all streams system tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1599/

Passed.

Github comments John's review Guozhang's follow up

mjsax · 2018-03-26T01:55:04Z

Added missing system test (missing combination of versions to be tested): https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1601/

Passed.

guozhangwang · 2018-03-26T20:35:17Z

+    @parametrize(old_version=str(LATEST_0_10_2), new_version=str(LATEST_0_11_0))
+    @parametrize(old_version=str(LATEST_0_10_2), new_version=str(DEV_VERSION))
+    @parametrize(old_version=str(LATEST_0_11_0), new_version=str(DEV_VERSION))
+    def test_simple_upgrade(self, old_version, new_version):


I'd suggest update the existing test directly in this PR.

Can you elaborate? Should we remove test_upgrade_downgrade_streams or merge this with test_upgrade_downgrade_streams ?

This is for your comment above:

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.

I'd suggest we consolidate them in this PR already, same for 1.1

bbejeck

Just some minor nits otherwise LGTM.

bbejeck · 2018-03-26T23:18:02Z

+         Thus, you need to update and recompile your code. Just swapping the Kafka Streams library jar file will not work and will break your application. </li>
+    <li> Upgrading from 0.10.0.x to 1.0.2 requires two rolling bounces with config <code>upgrade.from="0.10.0"</code> set for first upgrade phase
+        (cf. <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-268%3A+Simplify+Kafka+Streams+Rebalance+Metadata+Upgrade">KIP-268</a>).
+        As an alternative, and offline upgrade is also possible.


nit: As an alternative, an offline upgrade is also possible

bbejeck · 2018-03-26T23:19:49Z

+         Thus, you need to update and recompile your code. Just swapping the Kafka Streams library jar file will not work and will break your application. </li>
+    <li> Upgrading from 0.10.0.x to 0.10.2.2 requires two rolling bounces with config <code>upgrade.from="0.10.0"</code> set for first upgrade phase
+         (cf. <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-268%3A+Simplify+Kafka+Streams+Rebalance+Metadata+Upgrade">KIP-268</a>).
+         As an alternative, and offline upgrade is also possible.


nit: as above

bbejeck · 2018-03-26T23:20:26Z

         Thus, you need to update and recompile your code. Just swapping the Kafka Streams library jar file will not work and will break your application. </li>
+    <li> Upgrading from 0.10.0.x to 0.10.1.2 requires two rolling bounces with config <code>upgrade.from="0.10.0"</code> set for first upgrade phase
+         (cf. <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-268%3A+Simplify+Kafka+Streams+Rebalance+Metadata+Upgrade">KIP-268</a>).
+         As an alternative, and offline upgrade is also possible.


bbejeck · 2018-03-26T23:43:32Z

+        Starts 3 KafkaStreams instances with <old_version>, and upgrades one-by-one to <new_version>
+        """
+
+        self.topics = {


topics defined here and in next test, maybe move up to init

The general test run with 3 broker and creates topic with replication factor 3 an other config (and used init for setup) -- however, we need different single-broker setup here. If we cleanup and merge tests (what we should to right away or a follow up PR -- Guozhang suggested to do it in this PR) than I agree that it makes sense to unify the setup. WDYT?

bbejeck · 2018-03-26T23:48:30Z

Java 7 and Java 9 passed Java 8 failed exceeded rate limit

retest this please

bbejeck · 2018-03-30T15:47:08Z

Java 8 build aborted.

retest this please

vvcephei · 2018-03-30T16:28:44Z

JDK 7 failure (https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/12218/) was:

00:10:15.512 org.apache.kafka.clients.NetworkClientTest > testConnectionDelayDisconnected FAILED
00:10:15.512     java.lang.AssertionError: expected:<16032.0> but was:<22917.0>
00:10:15.512         at org.junit.Assert.fail(Assert.java:88)
00:10:15.512         at org.junit.Assert.failNotEquals(Assert.java:834)
00:10:15.512         at org.junit.Assert.assertEquals(Assert.java:553)
00:10:15.512         at org.junit.Assert.assertEquals(Assert.java:683)
00:10:15.512         at org.apache.kafka.clients.NetworkClientTest.testConnectionDelayDisconnected(NetworkClientTest.java:307)

JDK8 failure (https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/12207/consoleFull) was:

00:12:26.606 org.apache.kafka.clients.producer.internals.BufferPoolTest > testBlockTimeout FAILED
00:12:26.606     java.lang.AssertionError: available memory8
00:12:26.606         at org.junit.Assert.fail(Assert.java:88)
00:12:26.606         at org.junit.Assert.assertTrue(Assert.java:41)
00:12:26.606         at org.apache.kafka.clients.producer.internals.BufferPoolTest.testBlockTimeout(BufferPoolTest.java:190)

JDK9 is ongoing.

vvcephei

This PR LGTM. Not sure what's up with those failing tests, but they look unrelated.

mjsax · 2018-03-30T17:06:43Z

Updated this. Triggered system tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1634/

mjsax · 2018-03-31T04:17:40Z

Consolidated system tests as requested.

1634 failed partially. Fixed. Corresponding tests passed at https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1642/

Retriggered all upgrade tests again: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1643/ (passed)

Call for review.

mjsax · 2018-03-31T04:17:47Z

Retest this please

vvcephei

LGTM! Just a clarifying question.

Thanks for the consolidation.

vvcephei · 2018-04-02T14:40:43Z

+    #@parametrize(new_version=str(LATEST_0_10_1)) we cannot run this test until Kafka 0.10.1.2 is released
+    #@parametrize(new_version=str(LATEST_0_10_2)) we cannot run this test until Kafka 0.10.2.2 is released
+    #@parametrize(new_version=str(LATEST_0_11_0)) we cannot run this test until Kafka 0.11.0.3 is released
+    @parametrize(new_version=str(DEV_VERSION))


I did a double-take here... I think I get it now, but to check my understanding...

The plan is to wait until those patch releases, and then do something like :

@matrix(new_version=simple_upgrade_versions_metadata_version_2)

You're effectively slicing 0.10.0 out from the upgrade/downgrade test matrix because:

upgrading from 0.10.0 requires a different algorithm (double bounce)

downgrading to 0.10.0 is not supported

Right?

Yes. Upgrading from 0.10.0 requires two rolling bounces including the new config upgrade.from and thus we have this extra test for it -- this new config is only available in un-release code (for older non-dev branches) and thus, we cannot run this test for those versions atm.

We chould add a downgrade test though -- but it would be a new test method, too, as it also requires two rolling bounces but with different order of command compare to this test. It would be something like:

prepare all instances to bounce and set config upgrade.from="0.10.0"

do first round of rolling bounce -- in this rolling bounce you stay on current version and don't downgrade yet, but only prepare all instances for downgrading (via setting the config)

do a second round of rolling bounces downgrading to 0.10.0

I think it's not worth to add a downgrade test.

Agreed. We have no evidence that downgrade is necessary. We don't need to document it or test it unless or until we actually choose to support it.

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

38d357b

mjsax added the streams label Mar 23, 2018

mjsax requested review from dguy and guozhangwang March 23, 2018 23:48

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch from 6ae5b0b to f0531b3 Compare March 25, 2018 19:44

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch from f0531b3 to 5acbb1a Compare March 26, 2018 00:30

Update upgrade docs

c3a0c5f

Github comments John's review Guozhang's follow up

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch 3 times, most recently from 040aed6 to ad66b12 Compare March 26, 2018 01:07

mjsax mentioned this pull request Mar 26, 2018

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

Merged

Extends system tests for 1.0

1b94960

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch from ad66b12 to 1b94960 Compare March 26, 2018 18:57

guozhangwang reviewed Mar 26, 2018

View reviewed changes

bbejeck approved these changes Mar 26, 2018

View reviewed changes

mjsax mentioned this pull request Mar 27, 2018

MINOR: Fix typos in docs and address some missed review comments #4778

Merged

mjsax added 2 commits March 26, 2018 18:18

fix doc typos

642ccc7

remove generics

99ef392

mjsax mentioned this pull request Mar 27, 2018

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

Merged

move config

4f7c722

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch from 03aa5e1 to 263fcfd Compare March 29, 2018 23:37

vvcephei approved these changes Mar 30, 2018

View reviewed changes

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch 2 times, most recently from f7d669f to dd6c414 Compare March 30, 2018 17:05

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch 2 times, most recently from 07bb017 to 27ad139 Compare March 30, 2018 20:30

Consolidating system tests

35d3e92

mjsax force-pushed the kafka-6054-fix-upgrade-10 branch from 27ad139 to 35d3e92 Compare March 30, 2018 22:19

vvcephei approved these changes Apr 2, 2018

View reviewed changes

guozhangwang approved these changes Apr 2, 2018

View reviewed changes

mjsax merged commit 6a24f5b into apache:1.0 Apr 3, 2018

mjsax deleted the kafka-6054-fix-upgrade-10 branch June 5, 2018 23:48

Conversation

mjsax commented Mar 23, 2018

Uh oh!

mjsax commented Mar 23, 2018

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

bbejeck commented Mar 26, 2018

Uh oh!

bbejeck commented Mar 30, 2018

Uh oh!

vvcephei commented Mar 30, 2018

Uh oh!

vvcephei left a comment

Choose a reason for hiding this comment

Uh oh!

mjsax commented Mar 30, 2018

Uh oh!

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

Uh oh!

Uh oh!

mjsax commented Mar 31, 2018

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mjsax commented Mar 25, 2018 •

edited

Loading

mjsax commented Mar 26, 2018 •

edited

Loading

mjsax commented Mar 31, 2018 •

edited

Loading