Skip to content

MINOR: Need to have same wait as verify timeout broker upgrade downgrade#6127

Merged
guozhangwang merged 4 commits intoapache:trunkfrom
bbejeck:MINOR_need_to_have_same_wait_as_verify_timeout_broker_upgrade_downgrade
Jan 11, 2019
Merged

MINOR: Need to have same wait as verify timeout broker upgrade downgrade#6127
guozhangwang merged 4 commits intoapache:trunkfrom
bbejeck:MINOR_need_to_have_same_wait_as_verify_timeout_broker_upgrade_downgrade

Conversation

@bbejeck
Copy link
Copy Markdown
Member

@bbejeck bbejeck commented Jan 11, 2019

When I originally refactored the streams_upgrade_test#upgrade_downgrade_brokers test I removed the wait call which would wait for up 24 minutes for the SmokeTestDriver class to publish and verify all of its records.

Since most of the tests run in two minutes or less I set the monitor_log timeout to three minutes. However, the SmokeTestDriver#verify method allows up to six minutes to consume all records before verifying the monitor_log timeout needs to be greater than 6 minutes. I've set the timeout to 8 minutes.

Additionally, the steps needed to update the streams_upgrade_test should be documented as there are several components that need to get updated. So I've documented those steps here on the test as a giant comment.

I ran a branch builder (just for the broker upgrade tests) http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2019-01-11--001.1547174767--bbejeck--MINOR_need_to_have_same_wait_as_verify_timeout_broker_upgrade_downgrade--4fec6e4/report.html and all tests passed

Committer Checklist (excluded from commit message)

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

@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Jan 11, 2019

@guozhangwang, @mjsax, and @vvcephei for reviews

metadata_3_or_higher_versions = [str(LATEST_2_0), str(LATEST_2_1), str(DEV_VERSION)]

"""
After each any release this test needs to get updated, but this requires several steps
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.

After each release ... one should first check that the released version has been uploaded to https://s3-us-west-2.amazonaws.com/kafka-packages/ which is the url used by system test to download jars; anyone can verify that by calling curl https://s3-us-west-2.amazonaws.com/kafka-packages/kafka_$scala_version-$version.tgz to download the jar. If it is not uploaded yet, ping the dev@kafka mailing list to request it being uploaded.

nit: each any -> each.

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.

ack

@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Jan 11, 2019

updated this

@guozhangwang guozhangwang merged commit 3746bf2 into apache:trunk Jan 11, 2019
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.

LGTM. Thanks for the documentation as well.

@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Jan 11, 2019

Thanks @guozhangwang and @vvcephei

pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…ade (apache#6127)

When I originally refactored the streams_upgrade_test#upgrade_downgrade_brokers test I removed the wait call which would wait for up 24 minutes for the SmokeTestDriver class to publish and verify all of its records.

Since most of the tests run in two minutes or less I set the monitor_log timeout to three minutes. However, the SmokeTestDriver#verify method allows up to six minutes to consume all records before verifying the monitor_log timeout needs to be greater than 6 minutes. I've set the timeout to 8 minutes.

Additionally, the steps needed to update the streams_upgrade_test should be documented as there are several components that need to get updated. So I've documented those steps here on the test as a giant comment.

Reviewers: Guozhang Wang <wangguoz@gmail.com>
@bbejeck bbejeck deleted the MINOR_need_to_have_same_wait_as_verify_timeout_broker_upgrade_downgrade branch July 10, 2024 13:59
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.

3 participants