Skip to content

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

Merged
mjsax merged 10 commits intoapache:1.1from
mjsax:kafka-6054-fix-upgrade-11
Apr 6, 2018
Merged

KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0#4773
mjsax merged 10 commits intoapache:1.1from
mjsax:kafka-6054-fix-upgrade-11

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Mar 26, 2018

No description provided.

@mjsax mjsax added the streams label Mar 26, 2018
@mjsax mjsax requested review from dguy and guozhangwang March 26, 2018 01:55
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 26, 2018

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

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, those changes need to be added to this branch, too. System tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1605/

\cc @bbejeck @vvcephei

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Mar 26, 2018

@mjsax mjsax force-pushed the kafka-6054-fix-upgrade-11 branch from 1f51055 to fe8b254 Compare March 26, 2018 18:56
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to my other PR's comment, I'd suggest we update this test to get rid of using StreamsSmokeTestDriverService and use StreamsUpgradeTestJobRunnerService.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you elaborate? StreamsSmokeTestDriverService is used the generate the input data in this test and there is no alternative implementation, and StreamsUpgradeTestJobRunnerService is used already.

Copy link
Copy Markdown
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

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

This PR LGTM.

I do agree that we could consider consolidating system tests, but I also think we can call the scope on this PR big enough already. I think it's a better idea to make a separate task to step back and look at our whole testing strategy to try and globally optimize it.

Comment thread docs/streams/upgrade-guide.html Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these target versions be like 1.1.x?

(I'll not repeat this question on every occurrence)

mjsax added 7 commits April 2, 2018 22:28
Update upgrade docs
Github comments
John's review
Guozhang's follow up
remove generics

move config

Consolidating system tests
@mjsax mjsax force-pushed the kafka-6054-fix-upgrade-11 branch from fe8b254 to 1183029 Compare April 3, 2018 05:43
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 3, 2018

Rebased, cherry-picked missing changes from #4768, and address review comments. Triggered upgrade system test: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1648/

Call for review.

@guozhangwang
Copy link
Copy Markdown
Contributor

The system test 1648 failed, is it related?

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 3, 2018

I think so. Need to investigate... (maybe I introduced an issue rebasing or cherry-picking)

Copy link
Copy Markdown
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

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

This looks right to me.

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 4, 2018

Retriggered system tests: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1651/

1.1.0 release broke the tests -- needed to upgrade version for tests to 1.1.1-SNAPSHOT

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 4, 2018

The failing tests (test_upgrade_downgrade_brokers), all crash with

Exception in thread "main" java.lang.UnsupportedClassVersionError: kafka/Kafka : Unsupported major.minor version 52.0

on broker startup affecting version 1.0.1 only. We disabled this test in trunk. Should we disable for this PR -- issue seems to be unrelated?

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Apr 4, 2018

I think this error is saying that Kafka was compiled targeting JDK8 but you're attempting to run it with JDK7. If that sounds like the kind of thing we control in the system tests, I think we should fix it rather than disabling the test.

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Apr 4, 2018

@mjsax I forgot to @-mention you about this^^

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 4, 2018

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Apr 5, 2018

System test passed. Call for review.

@vvcephei Scala version was the issue -- I downgraded to use 2.11. System test are executed using JDK7 in branch builder.

@mjsax mjsax merged commit 7c313f4 into apache:1.1 Apr 6, 2018
ijuma added a commit to confluentinc/kafka that referenced this pull request Apr 25, 2018
* confluent/1.1: (60 commits)
  MINOR: Fix kafka-run-class for Java 10 (apache#4895)
  KAFKA-6772: Load credentials from ZK before accepting connections (apache#4867)
  KAFKA-6742: TopologyTestDriver error when dealing with stores from GlobalKTable
  MINOR: Mention that -1 disables retention by time (apache#4881)
  KAFKA-6790: Fix Streams processor node broken link (apache#4874)
  MINOR: Java 10 fixes so that the build passes (apache#4839)
  MINOR: Update Jackson to 2.9.5 (apache#4776)
  MINOR: Downgrade to Gradle 4.5.1 (apache#4791)
  MINOR: Java 9/10 fixes, gradle and minor deps update (apache#4725)
  KAFKA-6752: Enable unclean leader election metric (apache#4838)
  KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0 (apache#4773)
  KAFKA-6747 Check whether there is in-flight transaction before aborting transaction (apache#4826)
  KAFKA-6748: double check before scheduling a new task after the punctuate call (apache#4827)
  KAFKA-6739; Ignore headers when down-converting from V2 to V0/V1 (apache#4813)
  KAFKA-6728: Corrected the worker’s instantiation of the HeaderConverter
  KAFKA-6731: waitOnState should check the state to be the target start. (apache#4808)
  HOTFIX: Enforce a rebalance upon task migration (apache#4802)
  MINOR: Remove 1.2.0 changes from streams doc (apache#4784)
  MINOR: Update version numbers to 1.1.1-SNAPSHOT
  MINOR: Fix ReassignPartitionsClusterTest.testHwAfterPartitionReassignment test (apache#4781)
  ...
@mjsax mjsax deleted the kafka-6054-fix-upgrade-11 branch June 5, 2018 23:47
allenxwang pushed a commit to allenxwang/kafka that referenced this pull request Aug 24, 2018
…:1.1-nflx to 1.1-nflx

* commit '84eeea7fe4b3a64b84b87d231969acfee4fb7544':
  Fix a bug where the ReqeustPerSec API version hash map is not updated.
  KAFKA-6772: Load credentials from ZK before accepting connections (apache#4867)
  KAFKA-6742: TopologyTestDriver error when dealing with stores from GlobalKTable
  MINOR: Mention that -1 disables retention by time (apache#4881)
  KAFKA-6790: Fix Streams processor node broken link (apache#4874)
  MINOR: Java 10 fixes so that the build passes (apache#4839)
  MINOR: Update Jackson to 2.9.5 (apache#4776)
  MINOR: Downgrade to Gradle 4.5.1 (apache#4791)
  MINOR: Java 9/10 fixes, gradle and minor deps update (apache#4725)
  KAFKA-6752: Enable unclean leader election metric (apache#4838)
  KAFKA-6054: Fix upgrade path from Kafka Streams v0.10.0 (apache#4773)
  KAFKA-6747 Check whether there is in-flight transaction before aborting transaction (apache#4826)
  KAFKA-6748: double check before scheduling a new task after the punctuate call (apache#4827)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants