I like Jun's suggestion of adding ListGroups and DescribeGroup. It keeps the abstractions simple and avoids overloading existing requests.
On Wed, Oct 28, 2015 at 8:08 PM, Jason Gustafson <ja...@confluent.io> wrote: > 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 > > > > > > -- Thanks, Neha
