Skip to content

MINOR: Streams broker down flaky test - #6041

Merged
guozhangwang merged 5 commits into
apache:2.0from
bbejeck:MINOR_streams_broker_down_flaky_test
Dec 20, 2018
Merged

MINOR: Streams broker down flaky test#6041
guozhangwang merged 5 commits into
apache:2.0from
bbejeck:MINOR_streams_broker_down_flaky_test

Conversation

@bbejeck

@bbejeck bbejeck commented Dec 17, 2018

Copy link
Copy Markdown
Member

This PR addresses a few issues with this system test flakiness. I'll issue similar PRs for 2.1 and trunk as well.

  1. Need to grab the monitor before a given operation to observe logs for signal
  2. Relied too much on a timely rebalance and only sent a handful of messages.

I've updated the test and ran it here https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2141/ parameterized for 15 repeats all passed.

Committer Checklist (excluded from commit message)

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

topic,
max_messages=num_messages,
acks=1,
throughput=1000,

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.

set throughput to 1K per sec keep the volume records down, in some cases where the test runs long I've encountered OOM (default is 100K per sec)

time.sleep(broker_down_time_in_seconds)

self.kafka.start_node(node)
with processor.node.account.monitor_log(processor.LOG_FILE) as monitor:

@bbejeck bbejeck Dec 17, 2018

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.

This was one source of flakiness as before starting kafka and the subsequent discovery of group coordinator could happen before we can grab monitor and inspect log file. This is done throughout the test, but I'll only comment here.

self.wait_for_verification(processor, message, processor.STDOUT_FILE)
self.wait_for_verification(processor_2, message, processor_2.STDOUT_FILE)
self.wait_for_verification(processor_3, message, processor_3.STDOUT_FILE)
with processor.node.account.monitor_log(processor.LOG_FILE) as monitor_1:

@bbejeck bbejeck Dec 17, 2018

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.

Grab monitor for each respective streams client before starting kafka

timeout_sec=120,
err_msg=("Never saw '%s' on " % self.connected_message) + str(processor_3.node.account))

with processor.node.account.monitor_log(processor.STDOUT_FILE) as monitor_1:

@bbejeck bbejeck Dec 17, 2018

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.

Similar process here, grab monitor before producing any messages

with processor_2.node.account.monitor_log(processor_2.STDOUT_FILE) as monitor_2:
with processor_3.node.account.monitor_log(processor_3.STDOUT_FILE) as monitor_3:

self.assert_produce(self.inputTopic,

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.

break assertion of production to streams app and consuming from topic written to by streams into separate parts. I've done this in other parts of the test, but I won't comment on each section.

num_messages=self.num_messages,
timeout_sec=120)

monitor_1.wait_until(self.message,

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.

confirm each streams client is processing messages. Same as above, I've done this in other sections but I'll only call it out here.


# until KIP-91 is merged we'll only send 5 messages to assert Kafka Streams is running before taking the broker down
# After KIP-91 is merged we'll continue to send messages the duration of the test
self.assert_produce_consume(self.inputTopic,

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.

I'm not breaking the produce/consume assertion here as there is only one streams client in this test.

@bbejeck

bbejeck commented Dec 17, 2018

Copy link
Copy Markdown
Member Author

ping @guozhangwang, @mjsax, and @vvcephei for reviews

@guozhangwang

Copy link
Copy Markdown
Contributor

retest this please

@guozhangwang guozhangwang left a comment

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.

@bbejeck thanks for the PR! I've only a couple clarifications otherwise LGTM.

num_messages=self.num_messages,
timeout_sec=120)

self.wait_for_verification(processor_3, "processed9messages", processor_3.STDOUT_FILE)

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.

This is just a cleanup since this line is not necessary, right?

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.

Yes, that's correct.

self.wait_for_verification(processor, message, processor.STDOUT_FILE)
self.wait_for_verification(processor_2, message, processor_2.STDOUT_FILE)
self.wait_for_verification(processor_3, message, processor_3.STDOUT_FILE)
rebalance = "State transition from REBALANCING to RUNNING"

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.

Why only monitor on processor_3 not the previous two?

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.

Since processor 3 is the last streams client to join the group, once it has transitioned from REBALANCE to RUNNING the test is ready to proceed.

@guozhangwang

Copy link
Copy Markdown
Contributor

retest this please

2 similar comments
@guozhangwang

Copy link
Copy Markdown
Contributor

retest this please

@bbejeck

bbejeck commented Dec 19, 2018

Copy link
Copy Markdown
Member Author

retest this please

guozhangwang pushed a commit that referenced this pull request Dec 20, 2018
This PR addresses a few issues with this system test flakiness. This PR is a cherry-picked duplicate of #6041 but for the 2.1 branch, hence I won't repeat the inline comments here.

1. Need to grab the monitor before a given operation to observe logs for signal
2. Relied too much on a timely rebalance and only sent a handful of messages.
I've updated the test and ran it here https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2142/ parameterized for 15 repeats all passed.

Reviewers: Guozhang Wang <wangguoz@gmail.com>
@guozhangwang
guozhangwang merged commit ea9cb3c into apache:2.0 Dec 20, 2018
guozhangwang pushed a commit that referenced this pull request Dec 20, 2018
This PR addresses a few issues with this system test flakiness. This PR is a cherry-picked duplicate of #6041 but for the trunk branch, hence I won't repeat the inline comments here.

1. Need to grab the monitor before a given operation to observe logs for signal
2. Relied too much on a timely rebalance and only sent a handful of messages.
I've updated the test and ran it here https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2143/ parameterized for 15 repeats all passed.

Reviewers: Guozhang Wang <wangguoz@gmail.com>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
This PR addresses a few issues with this system test flakiness. This PR is a cherry-picked duplicate of apache#6041 but for the trunk branch, hence I won't repeat the inline comments here.

1. Need to grab the monitor before a given operation to observe logs for signal
2. Relied too much on a timely rebalance and only sent a handful of messages.
I've updated the test and ran it here https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2143/ parameterized for 15 repeats all passed.

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

2 participants