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
Fix test flakes in TestKafkaTransportProvider #599
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you, @akaritakai.
datastream-kafka/src/main/java/com/linkedin/datastream/kafka/KafkaTestUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Checking for the topic's existence using AdminUtils.topicExists(..) isn't enough to guarantee that the topic is ready for production, allowing a race condition to exist.
When we lose the race condition, the test fails like this:
As we can see, the broker is still setting everything up when the producer makes the metadata request, so the produce request fails. This failure also results in the test's KafkaProducer closing, making retries not possible in the current configuration.
A simple fix is to ensure that the topic is consumable (and thus is set up on the broker) before we try to produce to it, which is what this PR does.