Sorry for the delay, I have been struggling to come up with a nice solution:
I have updated the KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-347%3A++Enable+batching+in+FindCoordinatorRequest In summary, to solve the question Guozhang raised: "One tricky question is, how do we know if a higher version API has a batching optimization... a) One solution is to let the request's builder.build() return either ONE request or a LIST of requests. This is backward compatible. We can have a list of one single element. b) An alternative solution is to add extra fields in AbstractRequest.java including Boolean isBatchingEnable() and List<AbstractRequest> buildFromBatch(). This way will decouple the two different build functions. Then we update the send logic in doSend() correspondingly." You can read about these solutions in more details in this KIP. Thanks, Yishun On Fri, Aug 17, 2018 at 12:12 PM Yishun Guan <gyis...@gmail.com> wrote: > > Thanks for the clarification. I will address this in my KIP. > > On Fri, Aug 17, 2018, 12:06 PM Guozhang Wang <wangg...@gmail.com> wrote: >> >> Today we do have logic for auto down-conversion, but it is assuming a >> one-to-one mapping. The actual logic is here: >> >> https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/NetworkClient.java#L775 >> >> As you can see, NetworkClient maintains a "apiVersions" map that keeps which >> node supports which version. And then when sending the request to the node >> it will use this map to form the supported version of the request. >> >> But current logic do not consider that we may need multiple lower versioned >> requests to substitute a single higher versioned request, and that would be >> the logic your PR need to address. >> >> >> Guozhang >> >> On Fri, Aug 17, 2018 at 11:59 AM, Yishun Guan <gyis...@gmail.com> wrote: >>> >>> @Guozhang Wang One thing that I remain confused about (and forgive me if >>> you have explained this to me before), is that if we don't have any >>> transformation helpers (between versions) implemented before, how do we >>> deal with other incompatibility issues when we try to update requests and >>> bump up their versions? Or we never have this problem until this version >>> update and now that's why we need to implement a converter from V3 to V2? >>> >>> On Fri, Aug 17, 2018 at 10:18 AM Guozhang Wang <wangg...@gmail.com> wrote: >>>> >>>> Yishun, some more comments: >>>> >>>> 1. "All the coordinator ids " + "for this request": it should be "all the >>>> requested group ids looking for their coordinators" right? >>>> >>>> 2. I just thought about this a bit more, regarding "*Compatibility issues >>>> between old and new versions need to be considered, we should think about >>>> how to convert requests from a newer version to a old version."* >>>> >>>> >>>> One thing I realized is that for FindCoordinatorRequest, today both >>>> consumer / admin client would need it. I.e. to complete the KIP for >>>> compatibility, you'll have to implement this function along with this KIP, >>>> since otherwise consumer talking to an old broker will fail to. >>>> >>>> So I'd suggest you update the `Compatibility` section with a detailed >>>> proposal on how to let new versioned clients to talk to old versioned >>>> brokers. We've talked about some high-level implementation guidelines in >>>> the DISCUSS thread, which you can try it out and see if it works: i.e. by >>>> starting a Kafka broker with version 2.0, and then starting a consumer >>>> client with trunk (it will have a new version), and the added logic should >>>> make sure the consumer still proceeds normally with the compatibility logic >>>> that we are going to add. >>>> >>>> >>>> Guozhang >>>> >>>> On Thu, Aug 16, 2018 at 5:46 PM, Hu Xi <huxi...@hotmail.com> wrote: >>>> >>>> > +1 (non-binding) >>>> > >>>> > ________________________________ >>>> > 发件人: Yishun Guan <gyis...@gmail.com> >>>> > 发送时间: 2018年8月17日 8:14 >>>> > 收件人: dev@kafka.apache.org >>>> > 主题: [VOTE] KIP-347: Enable batching in FindCoordinatorRequest >>>> > >>>> > Hi all, >>>> > >>>> > I want to start a vote on this KIP: >>>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>> > 347%3A++Enable+batching+in+FindCoordinatorRequest >>>> > >>>> > Here is the discussion thread: >>>> > https://lists.apache.org/thread.html/fd727cc7d5b0956d64255c35d5ed46 >>>> > 403c3495a7052ba8ffbc55e084@%3Cdev.kafka.apache.org%3E >>>> > >>>> > Thanks everyone for your input! >>>> > >>>> > Best, >>>> > Yishun >>>> > >>>> >>>> >>>> >>>> -- >>>> -- Guozhang >> >> >> >> >> -- >> -- Guozhang
