Hi Chia-Ping, Thanks for the review and suggestions.
CI0: Thanks for the reminder. Update validVersions in ConsumerGroupDescribeRequest to 0-1. CI1: Yes, I use `ConsumerGroupMember#useClassicProtocol` to check whether a member in consumer group uses “classic” or “consumer” protocol. CI2: Yes, a member in share group always uses “share” protocol. CI3: Add a table to show meaning of “classic”, “consumer”, and “share” protocol. BTW, the vote thread is in https://lists.apache.org/thread/rb25tf75tzf4c7jqqldlo5jh9w8chsq6. Thanks, PoAn > On Nov 20, 2024, at 11:46 AM, Chia-Ping Tsai <chia7...@apache.org> wrote: > > hi PoAn > > CI0: We have to bump the version of ConsumerGroupDescribeRequest as well, so > server can distinguish the new and old behavior. > > CI1: the type of new filed is string, so I guess you plan to use > `ConsumerGroupMember#useClassicProtocol` [0] flag to return either "classic" > or "consumer", right? > > CI2: `MemberDescription` is used by `ShareGroupDescription` too, so the new > filed protocol in shared group is always "shared", right? > > CI3: Could you consider adding a table to show the value of the protocol > field in each case? Andrew has a beautiful table in KIP-1043 that lists all > possible protocol names. > > [0] > https://github.com/apache/kafka/blob/441a6d0b790f4a17b454caeea7588a6b90fbd9db/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/ConsumerGroupMember.java#L454 > > > Best, > Chia-Ping > > On 2024/11/19 15:45:16 PoAn Yang wrote: >> Hi David, >> >> Thanks for the reminder. >> >> DJ4: Add related content to the KIP. >> Bump validVersions in ConsumerGroupDescribeResponse to “0-1”. >> Add a new field “Protocol” to ConsumerGroupDescribeResponse. >> >> Thanks, >> PoAn >> >>> On Nov 19, 2024, at 3:57 PM, David Jacot <dja...@confluent.io.INVALID> >>> wrote: >>> >>> Hi PoAn, >>> >>> DJ4: Could you please also include the required changes to the >>> ConsumerGroupDescribed API in the public interface section? We basically >>> need to bump the version, add the new field, etc. >>> >>> Thanks, >>> David >>> >>> On Mon, Nov 18, 2024 at 5:40 AM PoAn Yang <yangp...@gmail.com> wrote: >>> >>>> Hi David, >>>> >>>> DJ4: Add a new field protocol to MemberDescription, >>>> so the command line tool can show protocol information when users describe >>>> members. >>>> >>>> If there is no further suggestion, I will start a vote thread today. >>>> >>>> Thanks, >>>> PoAn >>>> >>>>> On Nov 15, 2024, at 11:50 PM, David Jacot <dja...@confluent.io.INVALID> >>>> wrote: >>>>> >>>>> Hi, >>>>> >>>>> DJ2: Using "-" sounds good to me. >>>>> >>>>> DJ3: That seems reasonable to me. >>>>> >>>>> DJ4: Why not add it right now? I don't want to change the output of the >>>>> tool too many times. >>>>> >>>>> Best, >>>>> David >>>>> >>>>> On Fri, Nov 15, 2024 at 3:23 PM PoAn Yang <yangp...@gmail.com> wrote: >>>>> >>>>>> Hi David / Andrew, >>>>>> >>>>>> Thanks for review. Thanks Andrew for picking up kafka-share-groups.sh >>>>>> implementation. >>>>>> I will handle kafka-consumer-groups.sh. >>>>>> >>>>>> DJ3: After discussing with @Chia-Ping Tsai, we think that using new >>>> format >>>>>> is more clear. >>>>>> >>>>>> The new format will be like >>>>>> >>>> <topic-name1>:<partition-id1>,<partition-id2>;<topic-name2>:<partition-id1>,<partition-id2> >>>>>> >>>>>> Using colon(:) to concat topic name and partition IDs. >>>>>> Using comma(,) to concat partition IDs within same topic name. >>>>>> Using semicolon(;) to concat topic strings. >>>>>> >>>>>> AS3: Fix it with kafka-share-groups.sh. Thanks. >>>>>> >>>>>> If there is no further suggestion, I will start a vote thread next >>>> Monday. >>>>>> >>>>>> Thanks, >>>>>> PoAn >>>>>> >>>>>>> On Nov 14, 2024, at 9:05 PM, Andrew Schofield < >>>>>> andrew_schofield_j...@outlook.com> wrote: >>>>>>> >>>>>>> Hi PoAn, >>>>>>> DJ2: I was just going to comment that "-" would be a more appropriate >>>>>> missing value, but >>>>>>> you got there first. >>>>>>> >>>>>>> AS3: The examples for kafka-share-groups.sh include >>>>>> kafka-consumer-groups.sh in the >>>>>>> command line. >>>>>>> >>>>>>> If this is accepted in time, I'm happy to pick up the implementation of >>>>>> the share groups >>>>>>> part of this if it helps. >>>>>>> >>>>>>> Thanks, >>>>>>> Andrew >>>>>>> >>>>>>> ________________________________________ >>>>>>> From: Frank Yang <yangp...@gmail.com> >>>>>>> Sent: 14 November 2024 10:48 >>>>>>> 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 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 >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>> >>>>>> >>>> >>>> >> >>
