You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/06/18 15:36:15 UTC
[GitHub] [kafka] ryannedolan commented on a change in pull request #8894: KAFKA-9509: Add retries for mirrorClient consume records to fix flaky test
ryannedolan commented on a change in pull request #8894:
URL: https://github.com/apache/kafka/pull/8894#discussion_r442313614
##########
File path: connect/mirror/src/test/java/org/apache/kafka/connect/mirror/MirrorConnectorsIntegrationTest.java
##########
@@ -207,23 +212,45 @@ public void close() {
backup.stop();
}
+ // throw exception after 3 retries, and print expected error messages
+ private void assertEqualsWithConsumeRetries(final String errorMsg,
Review comment:
fwiw this doesn't adhere to kafka style guide (looks like Kafka Streams to me)
##########
File path: connect/mirror/src/test/java/org/apache/kafka/connect/mirror/MirrorConnectorsIntegrationTest.java
##########
@@ -207,23 +212,45 @@ public void close() {
backup.stop();
}
+ // throw exception after 3 retries, and print expected error messages
+ private void assertEqualsWithConsumeRetries(final String errorMsg,
+ final int numRecordsProduces,
+ final int timeout,
+ final ClusterType clusterType,
+ final String... topics) throws InterruptedException {
+ int retries = 3;
+ while (retries-- > 0) {
+ try {
+ int actualNum = clusterType == ClusterType.PRIMARY ?
+ primary.kafka().consume(numRecordsProduces, timeout, topics).count() :
Review comment:
these are really strange side-effects to have an an assert statement. I see what you are trying to do, but this is probably not the way to do it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org