Skip to content

Conversation

@seekskyworld
Copy link

@seekskyworld seekskyworld commented Jan 11, 2026

Summary

  • wait for the async InvalidTopicException instead of a single poll

Motivation

  • eliminate flakiness in
    ShareConsumerTest.testSubscribeOnInvalidTopicThrowsInvalidTopicException
    (KAFKA-19962)

Validation

  • ./gradlew :clients:clients-integration-tests:test --tests
    org.apache.kafka.clients.consumer.ShareConsumerTest.testSubscribeOnInvalidTopicThrowsInvalidTopicException

@github-actions github-actions bot added triage PRs from the community tests Test fixes (including flaky tests) clients small Small PRs labels Jan 11, 2026
@AndrewJSchofield AndrewJSchofield added KIP-932 Queues for Kafka ci-approved and removed triage PRs from the community labels Jan 12, 2026
@AndrewJSchofield
Copy link
Member

Thanks for the PR. I note that this test has been reliable for 2 weeks now without this PR. The test flakiness looks like it was due to a failure to close the group cleanly which was fixed by #21239. I will evaluate whether this fix is necessary in the presence of the PR I mentioned.

@seekskyworld
Copy link
Author

You may consider merging this request, as it will make the code more robust.

@AndrewJSchofield
Copy link
Member

@seekskyworld Looks like the build failed with a compile error. Please take a look.

@seekskyworld
Copy link
Author

@AndrewJSchofield Thanks — I’ve resubmitted the PR and fixed the compile error.

I’ve reviewed the code again, and I believe the change is still valuable as a defensive improvement even with #21239 in place.

Happy to close or adjust it if you feel it’s no longer needed.

@AndrewJSchofield
Copy link
Member

@seekskyworld Thanks for the PR, but I don't think it's necessary since #21239. If I look at develocity, I only see a failure on the 4.2 branch which does not have that PR. If I run locally, I can run 200 iterations with no failures. Looking at the existing test and the modified test in this PR, they aren't quite doing the same thing. The existing test waits in poll(Duration) until the exception is thrown, and I think we should retain a test for that logic. As a result, I think we should close this PR. Thanks for all the effort, but I feel this PR is no longer needed.

testPollThrowsInterruptExceptionIfInterrupted does however fail. If you're interested in fixing that one, I'd be very grateful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved clients KIP-932 Queues for Kafka small Small PRs tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants