Hello Yishun, Thanks for the proposed KIP. I made a pass over the wiki and here are some comments:
1. "DESCRIBE_GROUPS_RESPONSE_MEMBER_V0", why we need to encode the full schema for the "COORDINATOR_GROUPIDS_KEY_NAME" field? Note it includes a lot of fields such as member id that is not needed for this case. I think a "new ArrayOf(String)" for the group ids should be sufficient. 2. "schemaVersions" of the "FindCoordinatorRequest" needs to include FIND_COORDINATOR_REQUEST_V3 as well. 3. One thing you may need to consider is that, in the adminClient to handle broker compatibility, how to transform a new (v3) request to a bunch of (v2) requests if it detects the broker is still in old version and hence cannot support v3 request (this logic is already implemented via ApiVersionsRequest in AdminClient, but may need to be extended to handle one-to-many mapping of different versions). This is not sth. that you need to implement under this KIP, but I'd recommend you think about this earlier than later and see if it may affect this proposal. Guozhang On Sat, Aug 11, 2018 at 10:54 AM, Yishun Guan <gyis...@gmail.com> wrote: > Hi, thank you Ted! I have addressed your comments: > > 1. Added more descriptions about later optimization. > 2. Yes, I will implement the V3 later when this KIP gets accepted. > 3. Fixed. > > Thanks, > Yishun > > On Fri, Aug 10, 2018 at 3:32 PM Ted Yu <yuzhih...@gmail.com> wrote: > > > bq. this is the foundation of some later possible optimizations(enable > > batching in *describeConsumerGroups ...* > > > > *Can you say more why this change lays the foundation for the future > > optimizations ?* > > > > *You mentioned **FIND_COORDINATOR_REQUEST_V3 in the wiki but I don't see > it > > in PR.* > > *I assume you would add that later.* > > > > *Please read your wiki and fix grammatical error such as the following:* > > > > bq. that need to be make > > > > Thanks > > > > On Wed, Aug 8, 2018 at 3:55 PM Yishun Guan <gyis...@gmail.com> wrote: > > > > > Hi all, > > > > > > I would like to start a discussion on: > > > > > > KIP-347: Enable batching in FindCoordinatorRequest > > > https://cwiki.apache.org/confluence/x/CgZPBQ > > > > > > Thanks @Guozhang Wang <wangg...@gmail.com> for his help and patience! > > > > > > Thanks, > > > Yishun > > > > > > -- -- Guozhang
