MINOR: Rolling bounce upgrade fixed broker system test by bbejeck · Pull Request #4690 · apache/kafka

bbejeck · 2018-03-12T18:34:21Z

This introduces a system test where we do a rolling bounce upgrade of streams application instances against a fixed broker version. Each broker version starting with 0.10.2 up to trunk is tested. Streams applications start at 0.10.2 and do rolling upgrades up to trunk then do rolling downgrades back to 0.10.2.

This PR also includes a new utility class for Streams systems tests StreamsTest which extends KafkaTest and provides some convenience methods for working with VerifiableProducer and VerfiiableConsumer and validating results in log files.

There are other system tests streams_standby_replica_test.py and streams_broker_down_resilience_test.py using common functionality that can be pulled out and extend the StreamsTest class instead. I'll do that in a follow-on PR.

EDIT: Since the base class StreamsTest only exists in this PR so far, I've refactored the other tests as mentioned above.

I tested this by running branch builder https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1430/

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

bbejeck · 2018-03-12T18:35:35Z

@dguy @guozhangwang @mjsax @vvcephei

guozhangwang · 2018-03-12T22:43:52Z

@bbejeck you'll need to rebase from trunk, the jenkins failures are related to that.

guozhangwang · 2018-03-12T22:53:20Z

This line is a bit hard to understand, a prerequisite .. if ... but there is no is what.. to me, or is it concatenated from the previous sentence?

guozhangwang · 2018-03-12T22:59:50Z

nit: Maybe rename to StreamsTestDriver to be consistent with other services under streams.py.

Since this extends KafkaTest and isn't a service itself, what do you think about StreamsTestBase or BaseStreamsTest?

Good point, BaseStreamsTest is fine.

guozhangwang · 2018-03-12T23:03:54Z

Maybe just add LATEST_1_1 here as well, though we are not going to use it yet?

That causes some problems with the dockerfile, since we'd have to download our test jar from the mirror, but it's not there yet. In fact, 1.0.0 is the latest package in the mirror.

Those files are uploaded manually for the docker setup. I'll take care of it.

guozhangwang · 2018-03-12T23:04:57Z

Could we do @matrix(broker_version =["...", ...} as we did in simple benchmark?

bbejeck · 2018-03-13T14:55:24Z

rebased and updated per comments.

Also since we'd have to wait for this PR for merging to refactor the other tests, I've went ahead and updated the other tests mentioned above to remove the common functionality and extend from the base test class StreamsTest

Re-running system tests now.

bbejeck · 2018-03-13T22:32:00Z

updated with new system test results.

standby replica system test result
broker down system test results
rolling upgrade system test results

vvcephei · 2018-03-13T23:26:09Z

LGTM!

FYI: you'll also want to update the Dockerfile, but I have it updated in my PR, so I reckon it doesn't matter (#4689)
Actually, I forgot the vagrantfile, so between the two of us we have covered both ways to run the tests!

bbejeck · 2018-03-14T18:37:44Z

retest this please

mjsax

Just did a high level review atm.

mjsax · 2018-03-14T22:06:30Z

what about 0.10.1 and 1.1 ?

Added 0.10.1 I was thinking 0.10.2 was the first version we could do a rolling upgrade from. I've left out 1.1.0 for the moment as I was thinking this PR will be merged before the 1.1 release.

EDIT the package name of StreamsSoakTest is different in 0.10.1 I think @vvcephei ran into the same issue in #4689. The decision there was to leave out 0.10.1 so IMHO we should do the same here.

I see. That makes sense. I run into the same issue and put a workaround in #4636 -- we should clean this up, after we put proper testing for older versions into place as discussed in #4636.

mjsax · 2018-03-14T22:09:56Z

Don't we need to set version 0.10.1 for those initially? Otherwise, they will have trunk version

on line 84 of upgrade_and_verify_start method, we set the version with the first version from the array and start the node, so they all start with earliest specified.

bbejeck · 2018-03-15T14:39:35Z

updated this

…new test

…lass

bbejeck · 2018-03-16T00:31:01Z

rebased this

mjsax · 2018-03-16T23:16:08Z

I see. That makes sense. I run into the same issue and put a workaround in #4636 -- we should clean this up, after we put proper testing for older versions into place as discussed in #4636.

mjsax · 2018-03-16T23:20:54Z

+        self.update_processors_and_verify(self.streams_upgrade_versions)
+
+        # with order reversed now we test downgrading, verification run after each downgrade
+        self.update_processors_and_verify(self.streams_downgrade_versions)


Don't we need to stop all processor at the end?

bbejeck · 2018-03-19T18:47:15Z

updated this and kicked off new system test https://jenkins.confluent.io/job/system-test-kafka-branch-builder/1499/.

EDIT: Tests passed

bbejeck · 2018-03-19T19:17:29Z

ping @mjsax and @guozhangwang for merging

mjsax

LGMT.

mjsax · 2018-03-19T21:32:21Z

Retest this please

bbejeck · 2018-03-20T15:59:07Z

retest this please

bbejeck · 2018-03-21T01:42:54Z

retest this please

bbejeck · 2018-03-21T17:23:12Z

retest this please

bbejeck · 2018-03-21T20:28:07Z

retest this please

bbejeck · 2018-03-22T13:35:44Z

@guozhangwang @mjsax

All tests pass now, call for final review and merging

vvcephei

This lgtm.

Your implementation has got me thinking about refactoring the cross-version upgrade/downgrade test to set up the brokers just once and do a bunch of cross-grades like you've done. I think that test would run much faster that way. Something for when I'm on call, maybe...

mjsax · 2018-03-22T23:01:04Z

@vvcephei What you suggest will be part of KIP-268 implementation. Compare #4746 and #4758

mjsax · 2018-03-22T23:02:40Z

Merged to trunk. Thanks for the patch @bbejeck.

Reviewers: Guozhang Wang <guozhang@confluent.io>, John Roesler <john@confluent.io>, Matthias J. Sax <matthias@confluent.io>

guozhangwang reviewed Mar 12, 2018

View reviewed changes

guozhangwang added the streams label Mar 12, 2018

bbejeck force-pushed the MINOR_rolling_streams_upgrades_fixed_broker branch from 3d3dcc0 to 75bf4b3 Compare March 13, 2018 14:47

vvcephei mentioned this pull request Mar 14, 2018

MINOR: Streams system test fixes/updates #4689

Merged

3 tasks

mjsax requested review from guozhangwang and mjsax March 14, 2018 22:04

mjsax reviewed Mar 14, 2018

View reviewed changes

bbejeck added 11 commits March 15, 2018 20:25

add new kafka versions to base.sh, move functions to streams.py, add …

1df960b

…new test

MINOR: removed changes not needed for this PR

1eca0ae

Updates per comments, extracted common functionality into base test c…

a83b823

…lass

making call to wrong method

baa6b51

needed to add back old timeout period

f3f6724

renamed base test class to BaseStreamsTest

8d51a29

fixed order of topics for assertion, error string format

f7ea026

moved version list to class level, reused in matrix parameter

9086cda

increase timeout for after broker stop

255ef4e

add 0.10.1 latest version

b65a19c

removed 0.10.1 version

e457564

bbejeck force-pushed the MINOR_rolling_streams_upgrades_fixed_broker branch from 6805b2d to e457564 Compare March 16, 2018 00:30

mjsax reviewed Mar 16, 2018

View reviewed changes

updates per comments, stopping all processors at end of test

3dcfe8e

mjsax approved these changes Mar 19, 2018

View reviewed changes

vvcephei approved these changes Mar 22, 2018

View reviewed changes

mjsax merged commit 286216b into apache:trunk Mar 22, 2018

hejiefang pushed a commit to hejiefang/kafka that referenced this pull request Mar 26, 2018

MINOR: Rolling bounce upgrade fixed broker system test (apache#4690)

d5f9b48

Reviewers: Guozhang Wang <guozhang@confluent.io>, John Roesler <john@confluent.io>, Matthias J. Sax <matthias@confluent.io>

bbejeck deleted the MINOR_rolling_streams_upgrades_fixed_broker branch July 10, 2024 13:58

Conversation

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

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

bbejeck commented Mar 12, 2018

Uh oh!

guozhangwang commented Mar 12, 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

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 13, 2018

Uh oh!

bbejeck commented Mar 13, 2018

Uh oh!

vvcephei commented Mar 13, 2018

Uh oh!

bbejeck commented Mar 14, 2018

Uh oh!

mjsax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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 15, 2018

Uh oh!

bbejeck commented Mar 16, 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!

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

Uh oh!

Uh oh!

bbejeck commented Mar 19, 2018

Uh oh!

mjsax left a comment

Choose a reason for hiding this comment

Uh oh!

mjsax commented Mar 19, 2018

Uh oh!

bbejeck commented Mar 20, 2018

Uh oh!

bbejeck commented Mar 21, 2018

Uh oh!

bbejeck commented Mar 21, 2018

Uh oh!

bbejeck commented Mar 21, 2018

Uh oh!

bbejeck commented Mar 12, 2018 •

edited

Loading

bbejeck Mar 15, 2018 •

edited

Loading

bbejeck commented Mar 19, 2018 •

edited

Loading