KAFKA-9509: Fixing flakiness of MirrorConnectorsIntegrationTest.testReplication#8048
Merged
hachikuji merged 8 commits intoapache:trunkfrom Feb 7, 2020
Merged
KAFKA-9509: Fixing flakiness of MirrorConnectorsIntegrationTest.testReplication#8048hachikuji merged 8 commits intoapache:trunkfrom
hachikuji merged 8 commits intoapache:trunkfrom
Conversation
Contributor
|
ok to test |
hachikuji
reviewed
Feb 5, 2020
Contributor
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
Thanks, left a few comments.
kkonstantine
reviewed
Feb 5, 2020
Contributor
kkonstantine
left a comment
kkonstantine
left a comment
There was a problem hiding this comment.
Thanks @skaundinya15
This test is indeed flaky.
After a first pass on the PR I have a suggestion regarding your assertion code. We have a similar assertion elsewhere.
Contributor
Author
|
Thanks for the initial reviews @hachikuji and @kkonstantine! I've updated as per your comments by using the method posted by @kkonstantine and checking to see if the connector and its tasks were up after each connector is configured. @hachikuji Now that the logic has changed a bit, do you think it's okay to do the connector + task check after each connector is configured or do you think we should refactor it out so we can do it per |
kkonstantine
reviewed
Feb 6, 2020
kkonstantine
reviewed
Feb 6, 2020
Contributor
kkonstantine
left a comment
kkonstantine
left a comment
There was a problem hiding this comment.
Thanks @skaundinya15
Almost there I think. One more comment regarding how the assertion is validated.
Additionally, I'd also change the jenkins.sh file locally and would run this test repeatedly (like 100 times at least) to see if it ever fails again (locally and in a jenkins branch builder).
ryannedolan
approved these changes
Feb 6, 2020
Contributor
ryannedolan
left a comment
ryannedolan
left a comment
There was a problem hiding this comment.
Thanks, big improvement :)
hachikuji
reviewed
Feb 6, 2020
kkonstantine
approved these changes
Feb 6, 2020
Contributor
kkonstantine
left a comment
kkonstantine
left a comment
There was a problem hiding this comment.
Thanks for fixing this. LGTM
Contributor
Author
|
Thanks for the reviews @kkonstantine and @hachikuji! @hachikuji I just updated the PR with your suggested changes, so ready for another round of reviews whenever you are. |
hachikuji
reviewed
Feb 6, 2020
Contributor
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
Looks good, just a few small comments.
Contributor
|
ok to test |
Contributor
|
retest this please |
Contributor
|
ok to test |
Contributor
|
Checkstyle failed: |
Contributor
Author
|
@hachikuji Checkstyle issues should be fixed |
Contributor
|
ok to test |
Contributor
|
retest this please |
2 similar comments
Contributor
|
retest this please |
Contributor
|
retest this please |
hachikuji
approved these changes
Feb 7, 2020
Contributor
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the fix!
hachikuji
pushed a commit
that referenced
this pull request
Feb 7, 2020
…eplication (#8048) The test case `org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication` has shown to be increasingly flaky recently. This PR aims to make this test more deterministic. Specifically, the flakiness was due to a timing issue between the tasks not starting up in time for the test to start running. This PR remediates that by introducing a status check after every connector is started up. These status checks include that the connector is found on the connect cluster as well as there are tasks created and up and running for that connector. These checks are introduced before the test starts running so that there is a confidence that the connectors and tasks are started up correctly before the test runs. Reviewers: Konstantine Karantasis <konstantine@confluent.io>, Jason Gustafson <jason@confluent.io>
stanislavkozlovski
pushed a commit
to stanislavkozlovski/kafka
that referenced
this pull request
Feb 18, 2020
…eplication (apache#8048) The test case `org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication` has shown to be increasingly flaky recently. This PR aims to make this test more deterministic. Specifically, the flakiness was due to a timing issue between the tasks not starting up in time for the test to start running. This PR remediates that by introducing a status check after every connector is started up. These status checks include that the connector is found on the connect cluster as well as there are tasks created and up and running for that connector. These checks are introduced before the test starts running so that there is a confidence that the connectors and tasks are started up correctly before the test runs. Reviewers: Konstantine Karantasis <konstantine@confluent.io>, Jason Gustafson <jason@confluent.io>
ijuma
added a commit
to ijuma/kafka
that referenced
this pull request
Apr 28, 2020
…t-for-generated-requests * apache-github/trunk: (410 commits) KAFKA-8843: KIP-515: Zookeeper TLS support MINOR: Add missing quote for malformed line content (apache#8070) MINOR: Simplify KafkaProducerTest (apache#8044) KAFKA-9507; AdminClient should check for missing committed offsets (apache#8057) KAFKA-9519: Deprecate the --zookeeper flag in ConfigCommand (apache#8056) KAFKA-9509; Fixing flakiness of MirrorConnectorsIntegrationTest.testReplication (apache#8048) HOTFIX: Fix two test failures in JDK11 (apache#8063) DOCS - clarify transactionalID and idempotent behavior (apache#7821) MINOR: further InternalTopologyBuilder cleanup (apache#8046) MINOR: Add timer for update limit offsets (apache#8047) HOTFIX: Fix spotsbug failure in Kafka examples (apache#8051) KAFKA-9447: Add new customized EOS model example (apache#8031) KAFKA-8164: Add support for retrying failed (apache#8019) HOTFIX: checkstyle for newly added unit test KAFKA-9261; Client should handle unavailable leader metadata (apache#7770) MINOR: Fix typos introduced in KIP-559 (apache#8042) MINOR: Fixing null handilg in ValueAndTimestampSerializer (apache#7679) KAFKA-9113: Clean up task management and state management (apache#7997) MINOR: fix checkstyle issue in ConsumerConfig.java (apache#8038) KAFKA-9491; Increment high watermark after full log truncation (apache#8037) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA: https://issues.apache.org/jira/browse/KAFKA-9509
As the JIRA indicates,
org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplicationhas shown to be an increasingly flaky test recently. This PR aims to make this test more deterministic. Specifically, the flakiness was due to a timing issue between the tasks not starting up in time for the test to start running. This PR remediates that by introducing a status check after every connector is started up. These status checks include that the connector is found on the connect cluster as well as there are tasks created and up and running for that connector. These checks are introduced before the test starts running so that there is a confidence that the connectors and tasks are started up correctly before the test runs.Committer Checklist (excluded from commit message)