Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-16774: Delete flaky test since it is redundant #15978

Merged
merged 1 commit into from
May 17, 2024

Conversation

cadonna
Copy link
Contributor

@cadonna cadonna commented May 16, 2024

The test shouldCloseAllTaskProducersOnCloseIfEosEnabled() in StreamThreadTest is flaky with a concurrent modification exception. The concurrent modification exception is caused by the test itself because it starts a stream thread and at the same time the thread that executes the test calls methods on the stream thread. The stream thread was not designed for such a concurrency.
The tests verifies that under EOS the streams producer are closed during shutdown. Actually the test is not needed since we already have a test that verifies that when the stream thread shuts down also the task manager shuts down and for the tasks manager we have tests that verify that the producers are closed when the task manager shuts down.

This commit verifies that those tests are run under EOS and ALOS.

Committer Checklist (excluded from commit message)

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

The test shouldCloseAllTaskProducersOnCloseIfEosEnabled() in
StreamThreadTest is flaky with a concurrent modification exception.
The concurrent modification exception is caused by the test itself
because it starts a stream thread and at the same time the thread
that executes the test calls methods on the stream thread. The stream
thread was not designed for such a concurrency.
The tests verifies that under EOS the streams producer are closed
during shutdown. Actually the test is not needed since we already
have a test that verifies that when the stream thread shuts down
also the task manager shuts down and for the tasks manager we have
tests that verify that the producers are closed when the task manager
shuts down.

This commit verifies that those tests are run under EOS and ALOS.
new MockTime(1)
);

thread.start();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This combined with the calls on the thread below are the cause of the flakiness.

@cadonna cadonna requested review from chia7712 and mjsax May 16, 2024 15:10
@mjsax mjsax added streams tests Test fixes (including flaky tests) labels May 16, 2024
Copy link
Member

@mjsax mjsax 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 quick fix @cadonna!

@cadonna cadonna merged commit c58c21c into apache:trunk May 17, 2024
1 check failed
rreddy-22 pushed a commit to rreddy-22/kafka-rreddy that referenced this pull request May 24, 2024
The test shouldCloseAllTaskProducersOnCloseIfEosEnabled() in
StreamThreadTest is flaky with a concurrent modification exception.
The concurrent modification exception is caused by the test itself
because it starts a stream thread and at the same time the thread
that executes the test calls methods on the stream thread. The stream
thread was not designed for such a concurrency.
The tests verifies that under EOS the streams producer are closed
during shutdown. Actually the test is not needed since we already
have a test that verifies that when the stream thread shuts down
also the task manager shuts down and for the tasks manager we have
tests that verify that the producers are closed when the task manager
shuts down.

This commit verifies that those tests are run under EOS and ALOS.

Reviewer: Matthias J. Sax <matthias@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
streams tests Test fixes (including flaky tests)
Projects
None yet
2 participants