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-4034: Avoid unnecessary consumer coordinator lookup #1720
Conversation
Thanks for the PR. Can we please also add a test in either |
} | ||
|
||
public RetriableCommitFailedException(String message, Throwable t) { | ||
super(message, t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These exceptions are public classes so I don't think we can remove the existing constructors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I'll bring them back.
@ijuma Thanks for the review. I've added the integration test and attempted to clean up the coordinator lookup future logic. |
// the user is manually assigning partitions and managing their own offsets). | ||
fetcher.resetOffsetsIfNeeded(partitions); | ||
|
||
if (!subscriptions.hasAllFetchPositions()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that both calls to this function is covered by this check already (subscriptions.position(partition)
== null as similar effect), but I think no harm to add here just in case.
LGTM. |
LGTM |
Tests passed locally. Merging to trunk and 0.10.0 branches. |
Author: Jason Gustafson <jason@confluent.io> Reviewers: Guozhang Wang <wangguoz@gmail.com>, Ismael Juma <ismael@juma.me.uk> Closes #1720 from hachikuji/KAFKA-4034
Author: Jason Gustafson <jason@confluent.io> Reviewers: Guozhang Wang <wangguoz@gmail.com>, Ismael Juma <ismael@juma.me.uk> Closes apache#1720 from hachikuji/KAFKA-4034
Author: Jason Gustafson <jason@confluent.io> Reviewers: Guozhang Wang <wangguoz@gmail.com>, Ismael Juma <ismael@juma.me.uk> Closes apache#1720 from hachikuji/KAFKA-4034
No description provided.