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

KafkaSinkCluster: route ListGroups request #1790

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Oct 29, 2024

This turned out to be more complicated than I initially thought.

When AdminClient.listConsumerGroups is called, the java driver sends a ListGroups request to all brokers and then combines the responses, removing any duplicate entries.

To ensure that all groups are included shotover should also send requests to all brokers and combine them.
However we should avoid sending requests outside of shotovers rack to reduce the load of the operation.
This should still include all the results as the client will query all shotover nodes, and each shotover node will query a portion of the kafka brokers, ensuring all kafka nodes are queried.

A second commit was added to avoid running the new test case in the tests with a down shotover node, since that resulted in test failures as the down shotover node can not report its racks groups.
It might be easier to review that commit separately from the main commit.

@rukai rukai force-pushed the split_list_groups branch from 7dcfcf0 to 7b3719e Compare October 29, 2024 04:33
Copy link

codspeed-hq bot commented Oct 29, 2024

CodSpeed Performance Report

Merging #1790 will not alter performance

Comparing rukai:split_list_groups (03d1477) with main (e3bac43)

Summary

✅ 38 untouched benchmarks

@rukai rukai force-pushed the split_list_groups branch 3 times, most recently from 453adf5 to 74961b1 Compare October 29, 2024 21:04
@rukai rukai marked this pull request as ready for review October 29, 2024 21:56
@rukai rukai force-pushed the split_list_groups branch from 74961b1 to 03d1477 Compare October 30, 2024 00:51
@rukai rukai merged commit e09cd43 into shotover:main Oct 30, 2024
41 checks passed
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.

3 participants