Skip to content

KAFKA-17456: Make sure FindCoordinatorResponse get created before consumer #17404

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

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

frankvicky
Copy link
Contributor

@frankvicky frankvicky commented Oct 8, 2024

JIRA: KAFKA-17456

The incorrect order could lead to flaky (see KAFKA-17092 and KAFKA-17395). It would be nice that we fix all of them.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added consumer tests Test fixes (including flaky tests) clients small Small PRs labels Oct 8, 2024
Copy link
Contributor

@kirktrue kirktrue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @frankvicky for the PR!

If this deflakes the tests, that would be a big win as the KafkaConsumerTest is one of the flakiest tests we have. We need to run the corresponding tests many times (I don't have an exact number) for both consumers to ensure that we've fixed the flakiness and haven't introduced any regressions.

Thanks!

@frankvicky
Copy link
Contributor Author

I have looped the KafkaConsumerTest 360 times on my local machine.
Screenshot from 2024-10-16 21-35-07

@kirktrue
Copy link
Contributor

@frankvicky—where are we on merging this? It appears the build and all tests pass.

@frankvicky
Copy link
Contributor Author

@chia7712 Could you please take a look ?

@@ -2412,15 +2410,15 @@ public void testReturnRecordsDuringRebalance(GroupProtocol groupProtocol) throws
ConsumerMetadata metadata = createMetadata(subscription);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looped testReturnRecordsDuringRebalance and see the following error:

Gradle Test Run :clients:test > Gradle Test Executor 253 > KafkaConsumerTest > testReturnRecordsDuringRebalance(GroupProtocol) > "testReturnRecordsDuringRebalance(GroupProtocol).groupProtocol=CLASSIC" FAILED
    org.opentest4j.AssertionFailedError: expected: <11> but was: <0>
        at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
        at app//org.apache.kafka.clients.consumer.KafkaConsumerTest.testReturnRecordsDuringRebalance(KafkaConsumerTest.java:2440)

@frankvicky could you please check it?

Copy link

This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

@github-actions github-actions bot added the stale Stale PRs label Apr 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved clients consumer small Small PRs stale Stale PRs tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants