Hi everyone, I have add some changes to the KIP based on the Pull Request: https://github.com/apache/kafka/pull/4454#issuecomment-360553277 :
* Reduce the scope of the operations to Consumer Groups to avoid complexity of making assignments generic for Consumer and Connect groups. If Connect Group operations are required we can handle it in another KIP or add it here if needed. * Consider adding `deleteConsumerGroups` API once https://cwiki.apache.org/confluence/display/KAFKA/KIP-229%3A+DeleteGroups+API is merged. Looking forward to your feedback. If these changes look good we can keep the discussion on the PR. Cheers, Jorge. El dom., 7 ene. 2018 a las 21:00, Jorge Esteban Quilcate Otoya (< quilcate.jo...@gmail.com>) escribió: > Great! > > I have added `listGroupOffsets` to the KIP. > > If there are no additional feedback, VOTE thread is already open. > > Cheers, > Jorge > > > El mar., 2 ene. 2018 a las 17:49, Gwen Shapira (<g...@confluent.io>) > escribió: > >> 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 >> > > > > > > >>>>> >> > > > > > > >>>> >> > > > > > > >>> >> > > > > > > >> >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > >> > > >> > >> >
