Hi All, I've updated the KIP for ListGroups and DescribeGroup. This does simplify things quite a bit.
Since this is a blocker for 0.9 and there doesn't seem to be any major objections, let's go ahead and start the vote. Minor issues with the schemas can be addressed on the JIRA. Thanks, Jason On Thu, Oct 29, 2015 at 8:37 AM, Neha Narkhede <n...@confluent.io> wrote: > 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 >
