Hi David, Thanks for the review and suggestion! I would like to get this in AK 4.0 as well. I will do my best.
DJ1: Update KIP to put GROUP-EPOCH and TARGET-ASSIGNMENT-EPOCH before #MEMBERS. DJ2: I prefer to follow current missing column value “-“. (reference <https://github.com/apache/kafka/blob/18199028a672fd973ac37bf26316994babc2a6da/tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java#L92>) DJ3: Update KIP to use CURRENT-EPOCH CURRENT-ASSIGNMENT TARGET-EPOCH TARGET-ASSIGNMENT. Remove GROUP-EPOCH. For assignment value, it follows current output (reference <https://github.com/apache/kafka/blob/18199028a672fd973ac37bf26316994babc2a6da/tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java#L413-L418>). I think the form of `topicid-partitionid` is more clear. If we would like to use this form, I will update both output in kafka-consumer-groups.sh and kafka-share-groups.sh. DJ4: It looks like DescribeGroupsResponseData only has protocol type at group level. Both DescribeGroupsResponseData and ConsumerGroupDescribeResponseData don’t have protocol at member level. Could we use a followup to add it? DJ5: Update KIP to put LEADER-EPOCH before CURRENT-OFFSET. Thanks, PoAn > On Nov 14, 2024, at 3:43 PM, David Jacot <dja...@confluent.io.INVALID> wrote: > > Hi PoAn, > > Thanks for the KIP! I have a few minor comments/suggestions: > > DJ1: In the output of `--describe --verbose`, I would suggest putting > `GROUP-EPOCH` and `TARGET-ASSIGNMENT-EPOCH` before `#MEMBERS`. > DJ2: Continuing on the above, I assume that we will print out N/A for the > fields not supported by classic groups. Is this correct? > DJ3: In the output of `--describe --members --verbose`, I wonder if we > should use the following order and terms: CURRENT-EPOCH CURRENT-ASSIGNMENT > TARGET-EPOCH TARGET-ASSIGNMENT . I would remove the GROUP-EPOCH because it > is already in the group description. The value `(0)` used for the > assignment is incorrect. Here I suppose that we will print out the list of > partitions in the form of `topicid-partitionid`. > DJ4: Continuing on the above, I wonder if we should also add the protocol > used `classic` or `consumer`. For context, it is possible to have `classic` > members and `consumer` members in a `consumer` group during an online > upgrade from the classic protocol to the consumer protocol. Having this > information may be handy for administrators. What do you think? > DJ5: In the output of `--describe --offsets --verbose`, I would suggest > putting `LEADER-EPOCH` closer to `CURRENT-OFFSET`. > > It would be great if we could get this one in AK 4.0 as it changes the > output of the command. > > Thanks, > DJ > > On Fri, Nov 1, 2024 at 7:40 AM Frank Yang <yangp...@gmail.com> wrote: > >> Hi Sean / Andrew / Lianet, >> >> Thanks for all your review and suggestions. >> >> AS1, LM1, LM4: Change to add KIP-848 information when users give —verbose >> option. >> —describe —verbose: shows group epoch and target assignment epoch. >> —describe —members —verbose: shows above information, member epoch, and >> target assignment. >> >> AS2: Change to use MEMBER-EPOCH to align with KIP-848 definition. >> >> LM2: For classic group, it doesn’t have epoch, so I use Optional fields in >> ConsumerGroupDescription. >> For share group, it relies on KIP-848. It must have epoch, so I use int >> fields in ShareGroupDescription. >> >> LM3: Remove —state change. We can get group level information by —describe >> —verbose. >> >> SQ1: Add LEADER-EPOCH when users give —describe —offsets —verbose. >> >> Thanks. >> PoAn >> >>> On Nov 1, 2024, at 5:08 AM, Lianet M. <liane...@gmail.com> wrote: >>> >>> Hello Frank, thanks for the KIP! A few comments: >>> >>> LM1. I strongly agree with Andrew's suggestion of moving this into a >>> --verbose option. I expect someone would only attempt to make sense of >> the >>> epochs while debugging an issue, not while trying to view the group or >>> member's info. (Also member-epoch makes more sense to me, even more if we >>> end up agreeing in a --verbose). Related to both issues, my take is that >>> whoever is close to the protocol will expect member-epoch, whoever is not >>> will probably won't even need to see the epochs at all. >>> >>> LM2. Why are the epochs defined as Optional (don't we expect to always >> have >>> them? with 0 initially, defined on the broker side for the group ones, >> and >>> client side for the member) >>> >>> LM3. Why is the KIP including the –state option in the proposed changes? >> ( >>> >> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=327977411#KIP1099:Extendkafkaconsumergroupscommandlinetooltosupportnewconsumergroup---state >>> ). >>> I get that the output would change in that example, but it’s not because >> of >>> any change to the –state option itself. It's because of the change >> proposed >>> to the –described (required with the --state), and the changes to >>> --describe are already explained above (seems confusing, got me looking >> for >>> a change to the state filter which seemed unrelated). >>> >>> LM4. Group epoch and target assignment epoch are conceptually at the >> group >>> level. vs member epoch that lands at a member level. So wonder if we >> should >>> show them accordingly (ex. using the --verbose option) >>> –describe –verbose => shows group epoch and target assignment epoch >>> –describe –members –verbose => shows member epoch (along >>> with group epoch and target assignment epoch) >>> >>> Thanks! >>> >>> Lianet >>> >>> On Thu, Oct 31, 2024 at 7:30 AM Andrew Schofield < >>> andrew_schofield_j...@outlook.com> wrote: >>> >>>> Hi PoAn, >>>> Thanks for the KIP. I have a few comments. >>>> >>>> AS1: It seems to me that these new additions are most useful to people >>>> trying to understand >>>> the progress of rebalancing in quite some detail. The information is >>>> really not understandable >>>> for most users who do not have deep knowledge of KIP-848/932. >>>> >>>> As a result, I suggest for kafka-share-groups.sh that you add a >> --members >>>> --verbose option >>>> and only include the new information in the output for that option, >> rather >>>> than changing the >>>> non-verbose --members output. >>>> >>>> I also make a similar suggestion for kafka-consumer-groups.sh --members >>>> and only add the >>>> new information for the --verbose output. >>>> >>>> AS2: I strongly suggest that you use MEMBER-EPOCH instead of >>>> CONSUMER-EPOCH. >>>> I think it's more confusing not following the terminology of the KIPs. >> For >>>> one thing, >>>> we already have "member" in the admin client such as MemberDescription. >>>> It's not a >>>> new term. >>>> >>>> >>>> Thanks, >>>> Andrew >>>> ________________________________________ >>>> From: PoAn Yang <pay...@apache.org> >>>> Sent: 25 October 2024 13:55 >>>> To: dev@kafka.apache.org <dev@kafka.apache.org> >>>> Subject: Re: [DISCUSS] KIP-1099: Extend kafka-consumer-groups command >> line >>>> tool to support new consumer group >>>> >>>> Hi Lucas, >>>> >>>> Thanks for the review! >>>> >>>> 1) Yes, I add related change for kafka-share-groups.sh to the KIP. Could >>>> you take a look? Thanks for the suggestion. >>>> >>>> 2) We use CONSUMER-ID as member ID. If we use MEMBER-EPOCH here, users >> may >>>> confuse what is different between CONSUMER and MEMBER. >>>> >>>> Thanks, >>>> PoAn >>>> >>>> On 2024/10/23 13:28:17 Lucas Brutschy wrote: >>>>> Hi Frank, >>>>> >>>>> thanks for the KIP! >>>>> >>>>> 1) For consistency, should we do the same for >>>>> kafka-share-groups.sh, ShareGroupDescription, etc. ? Even if we do not >>>>> implement it right now if the share group implementation may still be >>>>> incomplete, it may make sense to include it in the KIP. >>>>> >>>>> 2) Why call it CONSUMER-EPOCH, not MEMBER-EPOCH? That would seem more >>>>> consistent. >>>>> >>>>> Cheers, >>>>> Lucas >>>>> >>>>> On Wed, Oct 23, 2024 at 2:41 PM Frank Yang <yangp...@gmail.com> wrote: >>>>> >>>>>> Hi all, >>>>>> >>>>>> I would like to kick off the discussion of KIP-1099. This KIP enhances >>>> the >>>>>> kafka-consumer-groups tools to include state which is introduced by >>>> KIP-848. >>>>>> >>>>>> KIP-1099: Extend kafka-consumer-groups command line tool to support >> new >>>>>> consumer group - Apache Kafka - Apache Software Foundation >>>>>> < >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1099%3A+Extend+kafka-consumer-groups+command+line+tool+to+support+new+consumer+group >>>>> >>>>>> cwiki.apache.org >>>>>> < >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1099%3A+Extend+kafka-consumer-groups+command+line+tool+to+support+new+consumer+group >>>>> >>>>>> [image: favicon.ico] >>>>>> < >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1099%3A+Extend+kafka-consumer-groups+command+line+tool+to+support+new+consumer+group >>>>> >>>>>> < >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1099%3A+Extend+kafka-consumer-groups+command+line+tool+to+support+new+consumer+group >>>>> >>>>>> >>>>>> Thank you, >>>>>> PoAn >>>>>> >>>>> >>>> >> >>
