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-8962: Use least_loaded_node() for describe_topics() #2000

Merged

Conversation

jeffwidman
Copy link
Collaborator

@jeffwidman jeffwidman commented Feb 16, 2020

In KAFKA-8962 the AdminClient.describe_topics() call was changed from
using the controller to using the least_loaded_node():
apache/kafka@3170896#diff-6869b8fccf6b098cbcb0676e8ceb26a7R1540

As a result, no metadata request/response processing needs to happen
through the controller, so it's safe to remove the custom
error-checking. Besides, I don't think this error-checking even added
any value because AFAIK no metadata response would return a
NotControllerError because the recipient broker wouldn't realize the
metadata request was intended for only the controller.

Originally our admin client was implemented using the least-loaded-node,
then later updated to the controller. So updating it back to
least-loaded node is a simple case of reverting the associated commits.

This reverts commit 7195f03.

This reverts commit 6e2978e.

This reverts commit f92889a.


This change is Reviewable

In KAFKA-8962 the `AdminClient.describe_topics()` call was changed from
using the controller to using the `least_loaded_node()`:
apache/kafka@3170896#diff-6869b8fccf6b098cbcb0676e8ceb26a7R1540

As a result, no metadata request/response processing needs to happen
through the controller, so it's safe to remove the custom
error-checking. Besides, I don't think this error-checking even added
any value because AFAIK no metadata response would return a
`NotControllerError` because the recipient broker wouldn't realize the
metadata request was intended for only the controller.

Originally our admin client was implemented using the least-loaded-node,
then later updated to the controller. So updating it back to
least-loaded node is a simple case of reverting the associated commits.

This reverts commit 7195f03.

This reverts commit 6e2978e.

This reverts commit f92889a.
@jeffwidman
Copy link
Collaborator Author

cc @TylerLubeck @jtribble for FYI.

@jeffwidman jeffwidman merged commit 3c3fdc1 into master Feb 16, 2020
@jeffwidman jeffwidman deleted the KAFKA-8962-use-least-loaded-node-for-admin-describe-topics branch February 16, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant