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 >>>> >>> >>
