You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "C0urante (via GitHub)" <gi...@apache.org> on 2024/07/04 19:11:30 UTC
[PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
C0urante opened a new pull request, #16525:
URL: https://github.com/apache/kafka/pull/16525
This test is failing very frequently because our retry logic doesn't handle `RebalanceNeededException` instances that are wrapped in an `ExecutionException`; see [Gradle Enterprise](https://ge.apache.org/scans/tests?search.rootProjectNames=kafka&search.startTimeMax=1720120130872&search.startTimeMin=1719460800000&search.timeZoneId=America%2FNew_York&tests.container=org.apache.kafka.connect.mirror.integration.DedicatedMirrorIntegrationTest&tests.test=testMultiNodeCluster()).
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #16525:
URL: https://github.com/apache/kafka/pull/16525#discussion_r1669065888
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/DedicatedMirrorIntegrationTest.java:
##########
@@ -354,8 +355,10 @@ private <T extends SourceConnector> void awaitTaskConfigurations(MirrorMaker mm,
.map(TaskInfo::config)
.allMatch(predicate);
} catch (Exception ex) {
Review Comment:
I think this was over-general in the first place, causing the typo. It should be an ExecutionException, and the retriable condition should be simplified. I don't see anything in this method that would throw a synchronous RetriableException.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on PR #16525:
URL: https://github.com/apache/kafka/pull/16525#issuecomment-2214224519
@gharris1727 would you mind taking a look at this quick fix when you have a moment?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on PR #16525:
URL: https://github.com/apache/kafka/pull/16525#issuecomment-2214937353
The latest change is trivial and I'm able to build and run all unit tests for the `:connect:mirror` module, and run the `DedicatedMirrorIntegrationTest` suite locally. Merging...
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante merged PR #16525:
URL: https://github.com/apache/kafka/pull/16525
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on PR #16525:
URL: https://github.com/apache/kafka/pull/16525#issuecomment-2214221139
I've run the affected test case 25 times per JDK on [a separate branch](https://github.com/apache/kafka/pull/16536); all 50 (25 runs per machine * 2 JDKs) runs passed.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Fix retry logic in DedicatedMirrorIntegrationTest::awaitTaskConfigurations [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on code in PR #16525:
URL: https://github.com/apache/kafka/pull/16525#discussion_r1669116891
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/DedicatedMirrorIntegrationTest.java:
##########
@@ -354,8 +355,10 @@ private <T extends SourceConnector> void awaitTaskConfigurations(MirrorMaker mm,
.map(TaskInfo::config)
.allMatch(predicate);
} catch (Exception ex) {
Review Comment:
True, I wanted to play it safe but upon reflection I also can't think of a single case where we'd want to handle anything except an ExecutionException that wraps a RebalanceNeededException. I've pushed an update to address this; LMKWYT
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org