listGroups and listGroupOffsets will make it a snap to transition the existing ConsumerGroups CLI to depend on client libraries only.
Thanks for adding them :) On Sun, Dec 31, 2017 at 1:39 PM Jorge Esteban Quilcate Otoya < quilcate.jo...@gmail.com> wrote: > Thanks all for your feedback, and sorry for late response. > > I'm considering the following: > > ```AdminClient.java > public abstract ListGroupsResult listGroups(ListGroupsOptions options); > > public ListGroupsResult listGroups() { > return listGroups(new ListGroupsOptions()); > } > > public ListGroupsResult listConsumerGroups(ListGroupsOptions options) { > //filtering groups by ConsumerProtocol.PROTOCOL_TYPE > } > > public ListGroupsResult listConsumerGroups() { > return listConsumerGroups(new ListGroupsOptions()); > } > ``` > > About `describeConsumerGroups`, I'm considering renaming to > `describeGroups` and rename `ConsumerGroupDescription` and > `ConsumerDescription` to `GroupDescription` to `MemberDescription`. > Not sure we need a deserializer, we can access `DescribeGroupsResponse` > members directly. > > As @dan says, I also think `listGroupOffsets` could be added to this KIP to > make it complete. > > I'm thinking about renaming this KIP to "Add Consumer Group operations to > Admin API". > > I'm updating the KIP accordingly. > > Cheers and happy 2018! > > Jorge. > > El mié., 13 dic. 2017 a las 19:06, Colin McCabe (<cmcc...@apache.org>) > escribió: > > > On Tue, Dec 12, 2017, at 09:39, Jason Gustafson wrote: > > > Hi Colin, > > > > > > They do share the same namespace. We have a "protocol type" field in > the > > > JoinGroup request to make sure that all members are of the same kind. > > > > Hi Jason, > > > > Thanks. That makes sense. > > > > > Very roughly what I was thinking is something like this. First we > > introduce an > > > interface for deserialization: > > > > > > interface GroupMetadataDeserializer<Metadata, Assignment> { > > > String protocolType(); > > > Metadata desrializeMetadata(ByteBuffer); > > > Assignment deserializeAssignment(ByteBuffer); > > > } > > > > > > Then we add some kind of generic container: > > > > > > class MemberMetadata<Metadata, Assignment> { > > > Metadata metadata; > > > Assignment assignment; > > > } > > > > > > Then we have two APIs: one generic and one specific to consumer groups: > > > > > > <M, A> Map<String, MemberMetadata<M,A>> describeGroup(String groupId, > > > GroupMetadataDeserializer<M, A> deserializer); > > > > > > Map<String, ConsumerGroupMetadata> describeConsumerGroup(String > groupId); > > > > > > (This is just a sketch, so obviously we can change them to use futures > or > > > to batch or whatever.) > > > > > > I think it would be fine to not provide a connect-specific API since > this > > > usage will probably be limited to Connect itself. > > > > Yeah, it probably makes sense to have a separation between describeGroup > > and describeConsumerGroup. > > > > We will have to be pretty careful with cross-version compatibility in > > describeConsumerGroup. It should be possible for an old client to talk > > to a new broker, and a new client to talk to an old broker. So we > > should be prepared to read data in multiple formats. > > > > I'm not sure if we need to have a 'deserializer' argument to > > describeGroup. We can just let them access a byte array, right? > > Theoretically they might also just want to check for the presence or > > absence of a group, but not deserialize anything. > > > > best, > > Colin > > > > > > > > Thanks, > > > Jason > > > > > > > > > On Mon, Dec 11, 2017 at 9:15 PM, Colin McCabe <cmcc...@apache.org> > > wrote: > > > > > > > Sorry... this is probably a silly question, but do Kafka Connect > groups > > > > share a namespace with consumer groups? If we had a separate API for > > > > Kafka Connect groups vs. Consumer groups, would that make sense? Or > > > > should we unify them? > > > > > > > > best, > > > > Colin > > > > > > > > > > > > On Mon, Dec 11, 2017, at 16:11, Jason Gustafson wrote: > > > > > Hi Jorge, > > > > > > > > > > Kafka group management is actually more general than consumer > groups > > > > > (e.g. > > > > > there are kafka connect groups). If we are adding these APIs, I > would > > > > > suggest we consider the more general protocol and how to expose > > > > > group-protocol-specific metadata. For example, it might be > > reasonable to > > > > > have both an API to access to the low-level bytes as well as some > > > > > higher-level convenience APIs for accessing consumer groups. > > > > > > > > > > Thanks, > > > > > Jason > > > > > > > > > > On Mon, Dec 4, 2017 at 4:07 PM, Matthias J. Sax < > > matth...@confluent.io> > > > > > wrote: > > > > > > > > > > > Jorge, > > > > > > > > > > > > is there any update regarding this KIP? > > > > > > > > > > > > > > > > > > -Matthias > > > > > > > > > > > > On 11/17/17 9:14 AM, Guozhang Wang wrote: > > > > > > > Hello Jorge, > > > > > > > > > > > > > > I made a pass over the wiki, and here are a few comments: > > > > > > > > > > > > > > 1. First, regarding to Tom's comment #2 above, I think if we > are > > only > > > > > > going > > > > > > > to include the String groupId. Then it is Okay to keep as a > > String > > > > than > > > > > > > using a new wrapper class. However, I think we could include > the > > > > > > > protocol_type returned from the ListGroupsResponse along with > the > > > > > > groupId. > > > > > > > This is a very useful information to tell which consumer groups > > are > > > > from > > > > > > > Connect, which ones are from Streams, which ones are > > user-customized > > > > etc. > > > > > > > With this, it is reasonable to keep a wrapper class. > > > > > > > > > > > > > > 2. In ConsumerDescription, could we also add the state, > > protocol_type > > > > > > > (these two are form DescribeGroupResponse), and the Node > > coordinator > > > > > > (this > > > > > > > may be returned from the AdminClient itself) as well? This is > > also > > > > for > > > > > > > information consistency with the old client (note that > > protocol_type > > > > was > > > > > > > called assignment_strategy there). > > > > > > > > > > > > > > 3. With 1) / 2) above, maybe we can rename > > "ConsumerGroupListing" to > > > > > > > "ConsumerGroupSummary" and make "ConsumerGroupDescription" an > > > > extended > > > > > > > class of the former with the additional fields? > > > > > > > > > > > > > > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 7, 2017 at 2:13 AM, Jorge Esteban Quilcate Otoya < > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > >> Hi Tom, > > > > > > >> > > > > > > >> 1. You're right. I've updated the KIP accordingly. > > > > > > >> 2. Yes, I have add it to keep consistency, but I'd like to > know > > what > > > > > > others > > > > > > >> think about this too. > > > > > > >> > > > > > > >> Cheers, > > > > > > >> Jorge. > > > > > > >> > > > > > > >> El mar., 7 nov. 2017 a las 9:29, Tom Bentley (< > > > > t.j.bent...@gmail.com>) > > > > > > >> escribió: > > > > > > >> > > > > > > >>> Hi again Jorge, > > > > > > >>> > > > > > > >>> A couple of minor points: > > > > > > >>> > > > > > > >>> 1. ConsumerGroupDescription has the member `name`, but > > everywhere > > > > else > > > > > > >> that > > > > > > >>> I've seen the term "group id" is used, so perhaps calling it > > "id" > > > > or > > > > > > >>> "groupId" would be more consistent. > > > > > > >>> 2. I think you've added ConsumerGroupListing for consistency > > with > > > > > > >>> TopicListing. For topics it makes sense because at well as > the > > name > > > > > > there > > > > > > >>> is whether the topic is internal. For consumer groups, though > > > > there is > > > > > > >> just > > > > > > >>> the name and having a separate ConsumerGroupListing seems > like > > it > > > > > > doesn't > > > > > > >>> add very much, and would mostly get in the way when using the > > API. > > > > I > > > > > > >> would > > > > > > >>> be interested in what others thought about this. > > > > > > >>> > > > > > > >>> Cheers, > > > > > > >>> > > > > > > >>> Tom > > > > > > >>> > > > > > > >>> On 6 November 2017 at 22:16, Jorge Esteban Quilcate Otoya < > > > > > > >>> quilcate.jo...@gmail.com> wrote: > > > > > > >>> > > > > > > >>>> Thanks for the feedback! > > > > > > >>>> > > > > > > >>>> @Ted Yu: Links added. > > > > > > >>>> > > > > > > >>>> KIP updated. Changes: > > > > > > >>>> > > > > > > >>>> * `#listConsumerGroups(ListConsumerGroupsOptions options)` > > added > > > > to > > > > > > >> the > > > > > > >>>> API. > > > > > > >>>> * `DescribeConsumerGroupResult` and > `ConsumerGroupDescription` > > > > classes > > > > > > >>>> described. > > > > > > >>>> > > > > > > >>>> Cheers, > > > > > > >>>> Jorge. > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> El lun., 6 nov. 2017 a las 20:28, Guozhang Wang (< > > > > wangg...@gmail.com > > > > > > >) > > > > > > >>>> escribió: > > > > > > >>>> > > > > > > >>>>> Hi Matthias, > > > > > > >>>>> > > > > > > >>>>> You meant "list groups" I think? > > > > > > >>>>> > > > > > > >>>>> Guozhang > > > > > > >>>>> > > > > > > >>>>> On Mon, Nov 6, 2017 at 11:17 AM, Matthias J. Sax < > > > > > > >>> matth...@confluent.io> > > > > > > >>>>> wrote: > > > > > > >>>>> > > > > > > >>>>>> The main goal of this KIP is to enable decoupling > > > > StreamsResetter > > > > > > >>> from > > > > > > >>>>>> core module. For this case (ie, using AdminClient within > > > > > > >>>>>> StreamsResetter) we get the group.id from the user as > > command > > > > line > > > > > > >>>>>> argument. Thus, I think the KIP is useful without > "describe > > > > group" > > > > > > >>>>>> command to. > > > > > > >>>>>> > > > > > > >>>>>> I am happy to include "describe group" command in the KIP. > > Just > > > > > > >> want > > > > > > >>> to > > > > > > >>>>>> point out, that there is no reason to insist on it IMHO. > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> -Matthias > > > > > > >>>>>> > > > > > > >>>>>> On 11/6/17 7:06 PM, Guozhang Wang wrote: > > > > > > >>>>>>> A quick question: I think we do not yet have the `list > > consumer > > > > > > >>>> groups` > > > > > > >>>>>>> func as in the old AdminClient. Without this `describe > > group` > > > > > > >> given > > > > > > >>>> the > > > > > > >>>>>>> group id would not be very useful. Could you include this > > as > > > > well > > > > > > >>> in > > > > > > >>>>> your > > > > > > >>>>>>> KIP? More specifically, you can look at > > > > > > >> kafka.admin.AdminClientfor > > > > > > >>>> more > > > > > > >>>>>>> details on the APIs. > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> Guozhang > > > > > > >>>>>>> > > > > > > >>>>>>> On Mon, Nov 6, 2017 at 7:22 AM, Ted Yu < > > yuzhih...@gmail.com> > > > > > > >>> wrote: > > > > > > >>>>>>> > > > > > > >>>>>>>> Please fill out Discussion thread and JIRA fields. > > > > > > >>>>>>>> > > > > > > >>>>>>>> Thanks > > > > > > >>>>>>>> > > > > > > >>>>>>>> On Mon, Nov 6, 2017 at 2:02 AM, Tom Bentley < > > > > > > >>> t.j.bent...@gmail.com> > > > > > > >>>>>> wrote: > > > > > > >>>>>>>> > > > > > > >>>>>>>>> Hi Jorge, > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Thanks for the KIP. A few initial comments: > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> 1. The AdminClient doesn't have any API like > > > > > > >>> `listConsumerGroups()` > > > > > > >>>>>>>>> currently, so in general how does a client know the > > group ids > > > > > > >> it > > > > > > >>> is > > > > > > >>>>>>>>> interested in? > > > > > > >>>>>>>>> 2. Could you fill in the API of > > DescribeConsumerGroupResult, > > > > > > >> just > > > > > > >>>> so > > > > > > >>>>>>>>> everyone knows exactly what being proposed. > > > > > > >>>>>>>>> 3. Can you describe the ConsumerGroupDescription class? > > > > > > >>>>>>>>> 4. Probably worth mentioning that this will use > > > > > > >>>>>>>>> DescribeGroupsRequest/Response, and also enumerating > the > > > > error > > > > > > >>>> codes > > > > > > >>>>>>>> that > > > > > > >>>>>>>>> can return (or, equivalently, enumerate the exceptions > > throw > > > > > > >> from > > > > > > >>>> the > > > > > > >>>>>>>>> futures obtained from the DescribeConsumerGroupResult). > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Cheers, > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Tom > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> On 6 November 2017 at 08:19, Jorge Esteban Quilcate > > Otoya < > > > > > > >>>>>>>>> quilcate.jo...@gmail.com> wrote: > > > > > > >>>>>>>>> > > > > > > >>>>>>>>>> Hi everyone, > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> I would like to start a discussion on KIP-222 [1] > based > > on > > > > > > >> issue > > > > > > >>>>> [2]. > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> Looking forward to feedback. > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> [1] > > > > > > >>>>>>>>>> https://cwiki.apache.org/confluence/pages/viewpage. > > > > > > >>>>>>>>> action?pageId=74686265 > > > > > > >>>>>>>>>> [2] https://issues.apache.org/jira/browse/KAFKA-6058 > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> Cheers, > > > > > > >>>>>>>>>> Jorge. > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>>> > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> -- > > > > > > >>>>> -- Guozhang > > > > > > >>>>> > > > > > > >>>> > > > > > > >>> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >