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-16720] AdminClient Support for ListShareGroupOffsets (2/2) #18671

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

Conversation

sjhajharia
Copy link
Contributor

@sjhajharia sjhajharia commented Jan 22, 2025

KAFKA-16720 aims at adding the support for the ListShareGroupOffsets AdminClient functionality. This PR is a followup on #18571. The previous PR had added the handling for ReadShareGroupStateSummaryRequest. This one continues to make changes for handling the DescribeShareGroupOffsetsRequest

The Flow diagram as per the KIP:
Screenshot 2025-01-23 at 6 29 16 PM

Key Changes in the PR:

  • Added handling of listShareGroupOffsets() in KafkaAdminClient and adding the corresponding tests to KafkaAdminClientTest.
  • To aid the above, we use DescribeShareGroupOffsetsHandler class. Here is where the request is created and it also houses the handling of the same and conversions needed. One thing to note here is that, I have specified thebuildBatchedRequest() method in such a way that the request is for one single groupId though it specifies a Set. I think we need to update the KIP to ensure that listShareGroupOffsets() takes a (String, ListShareGroupOffsetsSpec) instead of a Map given that the DescribeShareGroupOffsetsRequest can currently handle only a single groupId request. Here it diverges from the way listConsumerGroupOffsets is handled as the OffsetFetchRequest supports fetching offsets for multiple groups together (from v8)
  • Changes made to KafkaApis for handleDescribeShareGroupOffsetsRequest. One thing to be noted here is that the DescribeShareGroupOffsetsRequest uses the topic Name while the ReadShareGroupStateSummaryRequest needs the topic Uuid. Hence the conversion takes place in KafkaApis itself before relaying the same to the GroupCoordinator.
  • The GroupCoordinator now has describeShareGroupOffsets method where it handles the DesribeShareGroupOffsets request. Since here the call was to be made via the persister, code has been added for the same.
  • GroupCoordinatorService now additionally has the Persister. This is passed from the BrokerServer which passes the NoOpPersister in case KIP-932 isn't enabled on the clusters or DefaultStatePersister in case Share Groups are enabled.
  • Test changes for the various GroupCoordinator classes.
  • Changes to the ShareGroupCommand (and the tests) to make a call to the listShareGroupOffsets method to get the start offset.

@github-actions github-actions bot added triage PRs from the community core Kafka Broker clients labels Jan 22, 2025
@github-actions github-actions bot added the tools label Jan 23, 2025
@sjhajharia sjhajharia changed the title [WIP] KAFKA_16720 (Part 2): Implemenation of DescribeShareGroupOffsets [KAFKA-16720] AdminClient Support for ListShareGroupOffsets (2/2) Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clients core Kafka Broker tools triage PRs from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant