Hey Jun, Thanks for taking a look at this. Initially the thought was to make GroupMetadataRequest analogous to TopicMetadataRequest: one accepts an array of topics and returns topic metadata, the other accepts an array of groupIds and returns group metadata. However, the analogy doesn't quite fit since the current usage of GroupMetadataRequest in the consumer is really just to lookup the coordinator (perhaps a better name would be FindCoordinatorRequest?). The other issue is that we are trying to support simple group listing and detailed group inspection in the same request. Since group metadata can be relatively large, it would be nice to avoid having to send it when listing groups is all you want to do. So if we agree that we should not overload GroupMetadataRequest, then that leaves two options:
1. We can add a new DescribeGroup request which accepts an array of groupIds and includes an option to include/exclude member metadata (e.g. a "verbose" flag). 2. We can do your suggestion, which is to add a new ListGroups request for simple group listing, and DescribeGroup which only accepts a single groupId and always returns all member metadata. If there are no objections to adding the additional API requests, I probably favor your suggestion since it seems the simplest and least error-prone. However, I think the first option would also be reasonable if we wanted to keep the API surface small. In that case, it might make sense to rename GroupMetadataRequest to FindCoordinatorRequest, which would allow DescribeGroupRequest to be called GroupMetadataRequest. Then the analogy with TopicMetadataRequest would actually fit. Thanks, Jason On Wed, Oct 28, 2015 at 6:32 PM, Jun Rao <j...@confluent.io> wrote: > Jason, > > Thanks for the writeup. Perhaps we can have two new requests: > DescribeConsumerGroup and ListConsumerGroup. The former takes a list of > groups and returns a list of metadata (members, group metadata, member > metadata, etc) for each group. The latter takes nothing and just returns a > list of groups hosted on the broker. Using an empty list to represent "all" > can potentially generate a large response if there are many groups. > > Since this is marked as an 0.9.0 blocker, it would be great if other people > can review this KIP soon. > > Thanks, > > Jun > > > On Wed, Oct 28, 2015 at 3:37 PM, Ismael Juma <ism...@juma.me.uk> wrote: > > > On Wed, Oct 28, 2015 at 10:25 PM, Jason Gustafson <ja...@confluent.io> > > wrote: > > > > > Hey Ashish, > > > > > > Yeah, that's fine with me too. I thought people kind of frowned upon > the > > > use of an empty topic list to get all topics, but perhaps it's more of > an > > > issue at the user API level. > > > > > > > Yes, empty list to represent "all" is quite error-prone. In fact, we have > > one such bug in the authorization code in trunk right now (there is a PR > > open with a fix though). > > > > Ismael > > >
