You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "AndrewJSchofield (via GitHub)" <gi...@apache.org> on 2025/02/13 20:31:59 UTC

[PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

AndrewJSchofield opened a new pull request, #18894:
URL: https://github.com/apache/kafka/pull/18894

   This is the first PR to complete list of share group offsets. Primarily, it adds support for receiving `null` back from the admin client when offsets cannot be retrieved. The`kafka-share-groups.sh` tool no longer throws an exception in this situation.
   
   It also adds support for the final `--offsets --verbose` option for `kafka-share-groups.sh`.
   
   Note that the leader epoch is just a placeholder for now and another PR in this sequence will add it to the admin client response (it's already in the Kafka protocol response).
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on PR #18894:
URL: https://github.com/apache/kafka/pull/18894#issuecomment-2658962193

   @smjn The expected flow is as follows:
   * `Admin.listShareGroupOffsets` is called with an empty collection of `TopicPartition` in the `ListShareGroupOffsetsSpec`. This means "list all offsets for this share group".
   * The group coordinator will compile a list of the topic-partitions in the share group (share-partitions). This will be imperfect until the initialize work that you mention, but I anticipate that `GroupCoordinatorShare.sharePartitions(List<String>, long)` will be used and that the response from this will be improved (fixed) as a result of the intialize work that you mention.
   * The group coordinator will then issue `ReadShareGroupStateSummary` requests to the share coordinator shards responsible for those share-partitions' state.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #18894:
URL: https://github.com/apache/kafka/pull/18894


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on PR #18894:
URL: https://github.com/apache/kafka/pull/18894#issuecomment-2658899155

   > > @AndrewJSchofield could you please update the PR title? it is not matched to jira title.
   > 
   > The JIRA is "kafka-share-groups.sh --describe only shows the offset information when members exist". I think there will be 3 PRs before that is achieved. I need #18848 to land before I can address the second one which will be the majority of it.
   
   We probably need initialize to solve this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18894:
URL: https://github.com/apache/kafka/pull/18894#discussion_r1956203598


##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -250,7 +252,7 @@ TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInfo
                             groupId,
                             tp.getKey().topic(),
                             tp.getKey().partition(),
-                            earliestResult.get(tp.getKey())
+                            Optional.ofNullable(earliestResult.get(tp.getKey()))

Review Comment:
   @AndrewJSchofield thanks for the sharing. 
   
   >  but I went with consistency. It's modelled on ListConsumerGroupOffsetsResult
   
   fair enough



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18894:
URL: https://github.com/apache/kafka/pull/18894#discussion_r1955316657


##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -263,34 +265,53 @@ TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInfo
             return groupOffsets;
         }
 
-        private void printOffsets(TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInformation>>> offsets) {
+        private void printOffsets(TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInformation>>> offsets, boolean verbose) {
             offsets.forEach((groupId, tuple) -> {
                 ShareGroupDescription description = tuple.getKey();
                 Collection<SharePartitionOffsetInformation> offsetsInfo = tuple.getValue();
                 if (maybePrintEmptyGroupState(groupId, description.groupState(), offsetsInfo.size())) {
-                    String fmt = printOffsetFormat(groupId, offsetsInfo);
-                    System.out.printf(fmt, "GROUP", "TOPIC", "PARTITION", "START-OFFSET");
+                    String fmt = printOffsetFormat(groupId, offsetsInfo, verbose);
+
+                    if (verbose) {
+                        System.out.printf(fmt, "GROUP", "TOPIC", "PARTITION", "LEADER-EPOCH", "START-OFFSET");

Review Comment:
   Pardon me, are there plans to expand the information provided in verbose mode in the future?



##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -250,7 +252,7 @@ TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInfo
                             groupId,
                             tp.getKey().topic(),
                             tp.getKey().partition(),
-                            earliestResult.get(tp.getKey())
+                            Optional.ofNullable(earliestResult.get(tp.getKey()))

Review Comment:
   Out of curiosity, why is `null` used to represent "no start offset" instead of `-1`? I don't prefer to put `null`, as not all map implementations accept null values. Additionally, the `Map#get` method can return `null` even when no `null` values have been explicitly added to the map, which effectively conveys the absence of a start offset.
   
   please correct me if I misunderstand anything.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18894:
URL: https://github.com/apache/kafka/pull/18894#discussion_r1955782382


##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -263,34 +265,53 @@ TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInfo
             return groupOffsets;
         }
 
-        private void printOffsets(TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInformation>>> offsets) {
+        private void printOffsets(TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInformation>>> offsets, boolean verbose) {
             offsets.forEach((groupId, tuple) -> {
                 ShareGroupDescription description = tuple.getKey();
                 Collection<SharePartitionOffsetInformation> offsetsInfo = tuple.getValue();
                 if (maybePrintEmptyGroupState(groupId, description.groupState(), offsetsInfo.size())) {
-                    String fmt = printOffsetFormat(groupId, offsetsInfo);
-                    System.out.printf(fmt, "GROUP", "TOPIC", "PARTITION", "START-OFFSET");
+                    String fmt = printOffsetFormat(groupId, offsetsInfo, verbose);
+
+                    if (verbose) {
+                        System.out.printf(fmt, "GROUP", "TOPIC", "PARTITION", "LEADER-EPOCH", "START-OFFSET");

Review Comment:
   No plans so far. I'm essentially bringing `kafka-share-groups.sh` in line with `kafka-consumer-groups.sh` with the `--verbose` option introduced in KIP-1099. The leader epoch information has essentially the same benefit for understanding the group state as it has with a consumer group.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18894:
URL: https://github.com/apache/kafka/pull/18894#discussion_r1955794920


##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -250,7 +252,7 @@ TreeMap<String, Entry<ShareGroupDescription, Collection<SharePartitionOffsetInfo
                             groupId,
                             tp.getKey().topic(),
                             tp.getKey().partition(),
-                            earliestResult.get(tp.getKey())
+                            Optional.ofNullable(earliestResult.get(tp.getKey()))

Review Comment:
   I have analysed how `Admin.listConsumerGroupOffsets` behaves and copied that. I want someone with code that works with consumer groups to be able to move across to share groups with minimal friction. Conceptually, I agree with you, but I went with consistency. It's modelled on `ListConsumerGroupOffsetsResult`.
   
   I think there are two cases here. First, there might have been an error (most likely `TOPIC_AUTHORIZATION_FAILED`) for a particular requested topic-partition. Second, there might not have been an error, but there is no offset information. Both of these are `null`. Both "you cannot know the offset" and "there is no offset".



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-18761: Complete listing of share group offsets [1/N] [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on PR #18894:
URL: https://github.com/apache/kafka/pull/18894#issuecomment-2658691521

   > @AndrewJSchofield could you please update the PR title? it is not matched to jira title.
   
   The JIRA is "kafka-share-groups.sh --describe only shows the offset information when members exist". I think there will be 3 PRs before that is achieved. I need https://github.com/apache/kafka/pull/18848 to land before I can address the second one which will be the majority of it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org