Skip to content
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

KAFKA-17634: Tweak wakeup logic to match WakeupTrigger changes #17304

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

AndrewJSchofield
Copy link
Member

WakeupTrigger was refactored as a result of changes in AsyncKafkaConsumer. This PR makes the equivalent changes in ShareConsumerImpl.

Committer Checklist (excluded from commit message)

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

Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@AndrewJSchofield thanks for this patch. one question: how can we ensure the "match" is maintained in the future as both consumers become more complex? For example, should we refactor WakeupTrigger to reduce the duplicate code?

@AndrewJSchofield
Copy link
Member Author

@chia7712 There are essentially two new consumers based on request managers: AsyncKafkaConsumer and ShareConsumerImpl. They have different client interfaces, but they do share the infrastructure such as use of request managers, a network client delegate and wakeup trigger. Initially, the share consumer has its own heartbeat request manager and membership manager, with far too much code duplication. Now, these have been refactored so almost all of the code is in common.

For WakeupTrigger, there are some patterns that the caller uses to use it properly. ShareConsumerImpl wasn't quite doing it correctly so this PR is correcting that. I'm pretty comfortable with this now and I can't think immediately think of a refactor which will make it much better.

@chia7712 chia7712 merged commit 800de13 into apache:trunk Sep 30, 2024
9 checks passed
@AndrewJSchofield AndrewJSchofield deleted the KAFKA-17634 branch September 30, 2024 14:12
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
…#17304)

WakeupTrigger was refactored as a result of changes in AsyncKafkaConsumer. This PR makes the equivalent changes in ShareConsumerImpl.

Reviewers: Chia-Ping Tsai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants