Hi Lianet, Thanks for your comment. I’ve been digging more into the situation with describing groups in a broker with groups of multiple types. It’s a bit fiddly because of the introduction of the modern consumer groups by KIP-848 and the need for the admin client to cope with both kinds of consumer groups and older brokers.
If you use `kafka-consumer-groups.sh --describe --group MYSHARE` the output is: Error: Consumer group ‘MYSHARE’ does not exist. How does it get there? AdminClient.describeConsumerGroups is complicated. First, it uses the ConsumerGroupDescribe RPC which responds with GROUP_ID_NOT_FOUND (69) and an empty error message. The broker *could* fill in the error message to help with this situation but I don’t like that as a solution. Seems quite brittle. Then, it uses the DescribeGroups RPC in case it’s a classic consumer group. This responds with error code NONE (0) and makes the group look like a Dead consumer group. There is no error message field in that RPC at all, so we don’t have the option of using an error message to disambiguate. So, `kafka-consumer-groups.sh` thinks that it’s dealing with a dead consumer group and its output makes sense. My preferred course of action here is as you suggest to introduce the new error code, INVALID_GROUP_TYPE. If you use any of the following RPCs with the wrong type of group, you get this response: * ConsumerGroupDescribe * ShareGroupDescribe * ConsumerGroupHeartbeat * ShareGroupHeartbeat The remaining RPCs for consumer groups, such as ListOffsets and TxnOffsetCommit continue to use `GROUP_ID_NOT_FOUND`. Does that make sense? Any further comments? Thanks, Andrew > On 23 Jul 2024, at 17:26, Lianet M. <liane...@gmail.com> wrote: > > Hello Andrew, > > Bringing here the point I surfaced on the KIP-1071 thread: > > I wonder if at this point, where we're getting several new group types >> added, each with RPCs that are supposed to include groupId of a certain >> type, we should be more explicit about this situation. Maybe a kind of >> INVALID_GROUP_TYPE (group exists but not with a valid type for this RPC) vs >> a GROUP_ID_NOT_FOUND (group does not exist). Those errors would be >> consistently used across consumer, share, and streams RPCs whenever the >> group id is not of the expected type. > > > I noticed it on KIP-1071 but totally agree with you that it would make more > sense to consider it here. > > LM9. Regarding the point of introducing a new INVALID_GROUP_TYPE vs reusing > the existing INCONSISTENT_PROTOCOL_TYPE. My concern with reusing > INCONSISTENT_GROUP_PROTOCOL for errors with the group ID is that it mixes > the concepts of group type and protocol. Even though they are closely > related, we have 2 separate concepts (internally and presented in output > for commands), and the relationship is not 1-1 in all cases. Also, the > INCONSISTENT_GROUP_PROTOCOL is already used not only for protocol but also > when validating the list of assignors provided by a consumer in a > JoinGroupRequest. Seems a bit confusing to me already, so maybe better not > to add more to it? Just first thoughts. What do you think? > > Thanks, > Lianet > > On Fri, Jul 19, 2024 at 5:00 AM Andrew Schofield <andrew_schofi...@live.com> > wrote: > >> Hi Apoorv, >> Thanks for your comments. >> >> AM1: I chose to leave the majority of the administration for the different >> types of groups in their own tools. The differences between the group >> types are significant and I think that one uber tool that subsumes >> kafka-consumer-groups.sh, kafka-share-groups.sh and >> kafka-streams-application-reset.sh would be too overwhelming and >> difficult to use. For example, the output from describing a consumer group >> is not the same as the output from describing a share group. >> >> AM2: I think you’re highlighting some of the effects of the evolution >> of groups. The classic consumer group protocol defined the idea >> of protocol as a way of distinguishing between the various ways people >> had extended the base protocol - “consumer", “connect", and “sr" are the >> main ones I’ve seen, and the special “” for groups that are not using >> member assignment. >> >> For the modern group protocol, each of the proposed implementations >> brings its own use of the protocol string - “consumer”, “share” and >> “streams”. >> >> Now, prior to AK 4.0, in order to make the console consumer use the >> new group protocol, you set `--consumer-property group.protocol=consumer`. >> This tells a factory method in the consumer to use the AsyncKafkaConsumer >> (group type is Consumer, protocol is “consumer") as opposed to the >> LegacyKafkaConsumer (group type is Classic, protocol is “consumer”). >> In AK 4.0, the default group protocol will change and setting the property >> will not be necessary. The name of the configuration “group.protocol” >> is slightly misleading. In practice, this is most likely to be used pre-AK >> 4.0 >> by people wanting to try out the new consumer. >> >> AM3: When you try to create a share group and that group ID is already >> in use by another type of group, the error message is “Group CG1 is not >> a share group”. It exists already, with the wrong type. >> >> AM4: This KIP changes the error behaviour for `kafka-consumer-groups.sh` >> and `kafka-share-groups.sh` such that any operation on a group that finds >> the >> group type is incorrect reports “Error: Group XXX is not a consumer group” >> or >> equivalent for the other group types. This change makes things much easier >> to >> understand than they are today. >> >> AM5: That section is just clarifying what the behaviour is. I don’t think >> it had >> been written down before. >> >> Thanks, >> Andrew >> >>> On 18 Jul 2024, at 16:43, Apoorv Mittal <apoorvmitta...@gmail.com> >> wrote: >>> >>> Hi Andrew, >>> Thanks for the KIP. The group administration is getting difficult with >> new >>> types of groups being added and certainly the proposal looks great. >>> I have some questions: >>> >>> AM1: The current proposal defines the behaviour for listing and >> describing >>> groups, simplifying create for `kafka-share-groups.sh`. Do we want to >> leave >>> the other group administration like delete to respective groups or shall >>> have common behaviour defined i.e. leave to respective >>> kafka-consumer-groups.sh or kafka-share-groups.sh? >>> >>> AM2: Reading the notes on KIP-848, >>> >> https://cwiki.apache.org/confluence/display/KAFKA/The+Next+Generation+of+the+Consumer+Rebalance+Protocol+%28KIP-848%29+-+Early+Access+Release+Notes >> , >>> which requires `--consumer-property group.protocol=consumer` to enable >>> modern consumer group. But the listing for `classic` "type" also defines >>> "protocol" as `consumer` in some scenarios. Is it intended or `classic` >>> type should different protocol? >>> >>> AM3: The KIP adds behaviour for `kafka-share-groups.sh` which defines >> the >>> `--create` option. Though it simplifies group creation, what should be >> the >>> error behaviour when the group with the same name exists but not of >> "share" >>> group type? >>> >>> AM4: The GroupMetadataManager.java stores all groups in the same data >>> structure which means the name has to be unique across different group >>> types. Do you think we should also change the error message for existing >>> kafka-consumer-groups.sh and kafka-share-groups.sh to recommend using new >>> kafka-groups.sh for extensive groups list? As currently the individual >>> scripts will result in "Group already exists" while creating new groups >> but >>> listing with respective utility will not yield the group. >>> >>> AM5: The KIP defines compatibility considerations for the ListGroups RPC. >>> But it's unclear to me why it's needed as the client and server >> supporting >>> `kafka-groups.sh` will be post ListGroups v5 API anyways hence >> TypesFilter >>> will be supported in both client and server. Am I missing something here? >>> >>> Regards, >>> Apoorv Mittal >>> +44 7721681581 >>> >>> >>> On Wed, Jul 17, 2024 at 6:26 PM Andrew Schofield < >> andrew_schofi...@live.com> >>> wrote: >>> >>>> Hi Lianet, >>>> Thanks for your comments. >>>> >>>> LM5. Unfortunately, the protocol type has to be a string rather than >>>> an enumeration. This is because when people have created their own >>>> extensions of the classic consumer group protocol, they have chosen >>>> their own protocol strings. For example, the Confluent schema registry >>>> uses “sr” and there are other examples in the wild. >>>> >>>> LM6.1. It’s because of the difference between a parameterised >>>> type and a raw type. >>>> >>>> If I use: >>>> public class ListGroupsResult<T extends GroupListing> >>>> public class ListConsumerGroupsResult<ConsumerGroupListing> >>>> >>>> then ListGroupsResult (no type variable) is a raw type which does >>>> not provide a type for the type variable. This causes compiler warnings >>>> when the type is used, unless it’s used as >> ListGroupsResult<GroupListing>. >>>> >>>> However, this works better. >>>> public class AbstractListGroupsResult<T extends GroupListing> >>>> public class ListGroupsResult extends >>>> AbstractListGroupsResult<GroupListing> >>>> public class ListConsumerGroupsResult extends >>>> AbstractListGroupsResult<ConsumerGroupListing> >>>> >>>> I’ll change the KIP to use this. >>>> >>>> LM6.2. I was just pointing out a difference and you’re happy >>>> with it. That’s good. >>>> >>>> LM7. If you have a cluster with a mixture of classic and modern >>>> consumer groups, to list them all you could use this: >>>> >>>> bin/kafka-groups.sh --protocol consumer >>>> >>>> When there are no classic consumer groups, you could do: >>>> >>>> bin/kafka-groups.sh --group-type consumer >>>> >>>> However, this only gives a complete list if you don’t have any classic >>>> consumer groups. >>>> >>>> As a result, I suggested --consumer so you don’t need to know >>>> or care about the existence of classic and modern consumer groups. >>>> I think it helps, but you aren’t convinced I think, which tells me >>>> more thinking needed here. >>>> >>>> Maybe adding --share would help, so only power users would >>>> use --group-type or --protocol to deal with the more complicated >>>> cases. >>>> >>>> LM8. It’s just not clear. I was trying to make the output the same >>>> whether the group was created, or whether it already existed. In >>>> either case, there’s a share group in existence. The >>>> AlterShareGroupOffsets RPC doesn’t distinguish between the >>>> two cases in its response. >>>> >>>> Thanks, >>>> Andrew >>>> >>>>> On 16 Jul 2024, at 21:24, Lianet M. <liane...@gmail.com> wrote: >>>>> >>>>> Hello Andrew, thanks for considering the feedback. Some follow-ups and >>>>> other comments: >>>>> >>>>> LM4. Good point about the older RPC versions and therefore the >>>>> Optional<GroupType>, agreed. >>>>> >>>>> LM5. In GroupListing, should we use the existing >>>>> org.apache.kafka.clients.ProtocolType to represent the protocol >> (instead >>>> of >>>>> String). I don’t quite like the fact that the enum is inside the >>>>> GroupRebalanceConfig though, feels like it should be a first level >>>> citizen. >>>>> >>>>> >>>>> LM6. Regarding the changes around ListGroupResults with generics. >>>>> - LM6.1. What’s the need for keeping both, a base >>>>> AbstractListGroupsResult<T extends GroupListing> and the >> ListGroupsResult >>>>> extends AbstractListGroupsResult<GroupListing>? Would it work if >> instead >>>> we >>>>> simply have a single ListGroupsResult<T extends GroupListing> from >> which >>>>> specific groups would inherit? I'm thinking of this: >>>>> >>>>> public class *ListGroupsResult<T extends GroupListing>* -> this would >>>>> probably end up containing the implementation that currently exists in >>>>> ListConsumerGroupsResult for #all, #errors and #valid, that all group >>>> types >>>>> would be able to reuse if we use a generic T extends GroupListing >>>>> >>>>> >>>>> public class *ListConsumerGroupsResult extends >>>>> ListGroupsResult<ConsumerGroupListing>* -> slim impl, agreed >>>>> >>>>> - LM6.2. Related to the concern of the javadoc for >>>>> ListConsumerGroupsResult. This class will inherit 3 funcs (all, valid, >>>>> error), that have a common behaviour (and javadoc) regardless of the >>>>> generic type, so I expect it won’t be confusing in practice? We will >> end >>>> up >>>>> with the java doc for, let’s say, ListConsumerGroupsResult#all showing >>>> the >>>>> parent javadoc that aligns perfectly with what the #all does. If ever >> we >>>>> need a different behaviour/javadoc for any of the functions in the >> child >>>>> classes, we would have the alternative of overriding the func and >>>> javadoc. >>>>> Makes sense? Not sure if I’m missing other readability issues with the >>>>> javadocs you’re seeing. >>>>> >>>>> LM7. Looks better to me now with the added filter on the kafka-group.sh >>>> for >>>>> the protocol. But then, the new –consumer filter achieves the same as >>>>> –protocol CONSUMER right? If so, I wonder if it would just be simpler >> to >>>>> support the --protocol as a way to achieve this? (sharing your struggle >>>> on >>>>> how to get this right, but feels easier to discover and reason about >> the >>>>> more we have filters based on the output, and not made up of >>>>> combinations....let's keep iterating and we'll get there :) ) >>>>> >>>>> LM8. Is the output wrong (or just not clear) in this example? (It >> seemed >>>> to >>>>> me this was referring to the successful case where we create a new >> share >>>>> group, so I was expecting a "successfully created" kind of output) >>>>> >>>>> $ bin/kafka-share-groups.sh --bootstrap-server localhost:9092 >>>>> --create --group NewShareGroup >>>>> Share group 'NewShareGroup' exists. >>>>> >>>>> Thanks! >>>>> >>>>> Lianet >>>>> >>>>> On Mon, Jul 15, 2024 at 6:00 AM Andrew Schofield < >>>> andrew_schofi...@live.com> >>>>> wrote: >>>>> >>>>>> Hi Lianet, >>>>>> Thanks for your comments. >>>>>> >>>>>> LM1. Admin.listGroups() in principle needs to be able to return >>>>>> results from any version of the ListGroups RPC. The older versions do >>>>>> not contain the group type, so I think it’s reasonable to have >>>>>> Optional<GroupType>. I think there’s a difference between >>>>>> Optional.empty (I don’t know the group type) and >>>>>> GroupType.UNKNOWN (I know and do not understand the group type). >>>>>> As a result, I’ve changed the KIP to use Optional<GroupType>. >>>>>> >>>>>> I think that changing ConsumerGroupListing to extend >>>>>> GroupListing, and to do the same for ShareGroupListing makes sense. >>>>>> This does require that the overridden methods such as type() have >>>>>> signatures that match today’s definition of ConsumerGroupListing but >>>>>> that’s fine with the change I made to use Optional<GroupType> above. >>>>>> >>>>>> LM2. I think it’s possible to do something with generics along the >>>>>> lines you described. >>>>>> >>>>>> * public abstract class AbstractListGroupsResult<T extends >> GroupListing> >>>>>> * public class ListGroupsResult extends >>>>>> AbstractListGroupsResult<GroupListing> >>>>>> * public class ListConsumerGroupsResult extends >>>>>> AbstractListGroupsResult<ConsumerGroupListing> >>>>>> >>>>>> This does make the javadoc for ListConsumerGroupsResult less >>>>>> readable because its methods are now all inherited. The classes >>>>>> such as ListConsumerGroupsResult of course still have to exist >>>>>> but the implementation is very slim. >>>>>> >>>>>> What do you think of this? I haven’t yet updated the KIP in this >>>>>> case. >>>>>> >>>>>> LM3. I have been kicking around the syntax for kafka-group.sh >>>>>> for a while now and I too am not happy with the filters yet. I >>>> absolutely >>>>>> want to be able to display all consumer groups with a simple option, >>>>>> but history means that not a single filter under the covers. >>>>>> >>>>>> I suggest the following: >>>>>> --group-type which supports all group types >>>>>> --protocol which supports any string for protocol (there’s no >>>> enumeration) >>>>>> --consumer which matches all classic and modern consumer groups >>>>>> (and is thus a confection made by filtering on both group type and >>>>>> protocol). >>>>>> >>>>>> I’ve changed the KIP accordingly. Let me know what you think. >>>>>> >>>>>> Thanks, >>>>>> Andrew >>>>>> >>>>>> >>>>>>> On 12 Jul 2024, at 21:48, Lianet M. <liane...@gmail.com> wrote: >>>>>>> >>>>>>> Hey Andrew, thanks for the KIP, we definitely need visibility from a >>>>>> higher >>>>>>> level now that groups are growing. >>>>>>> >>>>>>> LM1. Should we have the existing >>>>>>> org.apache.kafka.clients.admin.ConsumerGroupListing extend the >>>>>> GroupListing >>>>>>> you’re proposing? ConsumerGroupListing already exists with a very >>>> similar >>>>>>> shape, and this would allow to set a common ground for the existing >>>> group >>>>>>> types, and the ones that are coming up (share groups and KS groups). >>>> Side >>>>>>> note, the existing ConsumerGroupListing has the type as Optional, but >>>>>> given >>>>>>> that the GroupType enum has an UNKNOWN type, I don’t quite get the >> need >>>>>> for >>>>>>> Optional and seems ok to me as you’re proposing it. >>>>>>> >>>>>>> LM2: if the point above makes sense, it would allow us to consider >>>>>> changing >>>>>>> the new ListGroupResult you’re proposing to make it generic and >>>>>> potentially >>>>>>> reused by all group types: >>>>>>> >>>>>>> >>>>>>> public class ListGroupsResult { >>>>>>> >>>>>>> public KafkaFuture<Collection<? extends GroupListing>> all() >>>>>>> >>>>>>> public KafkaFuture<Collection<? extends GroupListing>> valid() { } >>>>>>> >>>>>>> public KafkaFuture<Collection<Throwable>> errors() { } >>>>>>> >>>>>>> } >>>>>>> >>>>>>> Makes sense? With this, maybe we won’t need specific result classes >> for >>>>>>> each group (like the existing ListConsumerGroupsResult), given that >> in >>>>>> the >>>>>>> end it’s just a wrapper around the GroupListing (which is what each >>>> group >>>>>>> type would redefine). >>>>>>> >>>>>>> >>>>>>> LM3. I get how you're playing with the filters for group types and >>>>>>> protocol, but then I find it confusing how we end up with filters >> that >>>> do >>>>>>> not match the output ( --group-type that matches the protocol from >> the >>>>>>> output and not the type for "consumer" example). What about having >> the >>>>>>> –group-type filter on the actual GroupType field of the RPC response >>>>>> (shown >>>>>>> in the cmd line output as TYPE); and add a –protocol-type that would >>>>>> filter >>>>>>> on the ProtocolType field of RPC response (shown in the cmd line >>>> output >>>>>> as >>>>>>> PROTOCOL). We would have the filters aligned with the output for all >>>>>> cases, >>>>>>> which seems more consistent. >>>>>>> >>>>>>> Thanks! >>>>>>> >>>>>>> Lianet >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Jun 6, 2024 at 8:16 AM Andrew Schofield < >>>>>> andrew_schofi...@live.com> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Kirk, >>>>>>>> Thanks for your comments. >>>>>>>> >>>>>>>> 1. I’m a big fan of consistency in these things and the method >>>>>> signatures >>>>>>>> match >>>>>>>> ListConsumerGroupsResult and ListShareGroupsResult. >>>>>>>> >>>>>>>> 2. Yes, client-side filtering. >>>>>>>> >>>>>>>> 3. I didn’t offer “classic” as an option for --group-type. I’ve >> kicked >>>>>> the >>>>>>>> options >>>>>>>> around in my mind for a while and I decided that using --group-type >>>> as a >>>>>>>> way of >>>>>>>> filtering types in a way that a normal user would understand them >> was >>>> a >>>>>>>> good >>>>>>>> place to start. For example, I didn’t have `--protocol consumer` for >>>>>>>> consumer groups >>>>>>>> and `--group-type share` for share groups, even though that’s >>>>>> technically >>>>>>>> more >>>>>>>> correct. >>>>>>>> >>>>>>>> Since KIP-848, the set of consumer groups is actually formed from >>>> those >>>>>>>> which >>>>>>>> use the classic protocol and those which use the modern protocol. >> This >>>>>> tool >>>>>>>> gives you both together when you use `--group-type consumer`, which >> is >>>>>>>> exactly >>>>>>>> what kafka-consumer-groups.sh does. >>>>>>>> >>>>>>>> Do you think - -group-type classic is helpful? It would give a list >> of >>>>>> all >>>>>>>> groups using >>>>>>>> any variant of the classic group protocol. I can easily add it. >>>>>>>> >>>>>>>> 4, 5. Yes, maybe the wording of the message could improve. These >>>> things >>>>>>>> are always >>>>>>>> tricky. I went with “Group CG1 is not a share group.” because it >>>> doesn’t >>>>>>>> require the tool >>>>>>>> to interpret the group type in order to generate the message. >>>>>>>> >>>>>>>> Imagine this scenario. You are using kafka-share-groups.sh >> --describe >>>>>> and >>>>>>>> you’ve >>>>>>>> used the group ID of a consumer group. Here are some options: >>>>>>>> >>>>>>>> a) “Group CG1 is not a share group.” >>>>>>>> b) “Incorrect group type (Consumer). Group CG1 is not a share >> group.” >>>>>>>> c) “Group CG1 has the wrong type for this operation. It is not a >> share >>>>>>>> group." >>>>>>>> >>>>>>>> I don’t think “There is already a (consumer) group named ‘CG1’” is >>>> quite >>>>>>>> right. >>>>>>>> >>>>>>>> Any preference? >>>>>>>> >>>>>>>> 6. Yes, it is a change in behaviour which is why I mention it in the >>>>>> KIP. >>>>>>>> Personally, I think that’s OK because the existing message is >>>> misleading >>>>>>>> and could definitely cause frustration. Let’s see what other >> reviewers >>>>>>>> think. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Andrew >>>>>>>> >>>>>>>>> On 6 Jun 2024, at 00:44, Kirk True <k...@kirktrue.pro> wrote: >>>>>>>>> >>>>>>>>> Hi Andrew, >>>>>>>>> >>>>>>>>> Thanks for the KIP! I don’t have much experience as a Kafka >> operator, >>>>>>>> but this seems like a very sane proposal. >>>>>>>>> >>>>>>>>> Questions & comments: >>>>>>>>> >>>>>>>>> 1. Do you think the ListGroupsResult.all() method is a bit of a >>>>>>>> potential ‘foot gun’? I can imagine cases where developers reach for >>>>>> that >>>>>>>> without understanding its potential of throwing errors. It could >> lead >>>> to >>>>>>>> cases where all() works in development but not in production. >>>>>>>>> >>>>>>>>> 2. Based on the LIST_GROUPS RPC, it appears that filtering is all >>>>>>>> performed client side, correct? (I know that’s not specific to this >>>> KIP, >>>>>>>> but just want to make sure I understand.) >>>>>>>>> >>>>>>>>> 3. For kafka-groups.sh --list, is ‘classic’ valid for --group-type? >>>> If >>>>>>>> so, should we allow users of kafka-groups.sh --list to provide >>>> multiple >>>>>>>> --group-type arguments? >>>>>>>>> >>>>>>>>> 4. In the last kafka-share-groups.sh --create example >>>>>> (“ConsumerGroup”), >>>>>>>> the error simply states that “Group 'ConsumerGroup’ is not a share >>>>>> group.” >>>>>>>> I’m assuming that’s the case where the user gets a failure when >>>> there’s >>>>>>>> already a group named “ConsumerGroup”, right? If so, the error >> should >>>> be >>>>>>>> something like “There is already a (consumer) group named >>>>>> ’ConsumerGroup’”. >>>>>>>>> >>>>>>>>> 5. In the last kafka-share-groups.sh --describe example, how hard >> is >>>> it >>>>>>>> to add the type of group that CG1 is, just for a bit of clarity for >>>> the >>>>>>>> user? >>>>>>>>> >>>>>>>>> 6. In the kafka-consumer-groups.sh section, it states "if that >> group >>>>>>>> exists but is not a consumer group, the command fails with a message >>>>>>>> indicating that the group type is incorrect, rather than the >> existing >>>>>>>> message that the group does not exist.” That sounds like a change >> that >>>>>>>> could trip up some brittle scripts somewhere, but I don’t know if >>>>>> that’s a >>>>>>>> serious problem. >>>>>>>>> >>>>>>>>> Thanks! >>>>>>>>> >>>>>>>>>> On Jun 4, 2024, at 10:08 AM, Andrew Schofield < >>>>>>>> andrew_schofi...@live.com> wrote: >>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> I would like to start a discussion thread on KIP-1043: >>>> Administration >>>>>>>> of groups. This KIP enhances the command-line tools to make it >> easier >>>> to >>>>>>>> administer groups on clusters with a variety of types of groups. >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1043%3A+Administration+of+groups >>>>>>>>>> >>>>>>>>>> Thanks. >>>>>>>>>> Andrew >> >> >>
