Hi, I am looking into AdminClient.scala and AdminClient.java, and also looking into ApiVersionRequest.java and ApiVersionResponse.java, but I don't see anywhere contains to logic of the one-to-one mapping from version to version, am i looking at the right place?
On Mon, Aug 13, 2018 at 1:30 PM Guozhang Wang <wangg...@gmail.com> wrote: > Regarding 3): Today we do not have this logic with the existing client, > because defer the decision about the version to use (we always assume that > an new versioned request need to be down-converted to a single old > versioned request: i.e. an one-to-one mapping), but in principle, we should > be able to modify the client make it work. > > Again this is not necessarily need to be included in this KIP, but I'd > recommend you to look into AdminClient implementations around the > ApiVersionRequest / Response and think about how that logic can be modified > in the follow-up PR of this KIP. > > > > Guozhang > > On Mon, Aug 13, 2018 at 12:55 PM, Yishun Guan <gyis...@gmail.com> wrote: > > > @Guozhang, thank you so much! > > 1. I agree, fixed. > > 2. Added. > > 3. I see, that is something that I haven't think about. How does Kafka > > handle other api's different version problem now? So we have a specific > > convertor that convect a new version request to a old version one for > each > > API (is this what the ApiVersionsRequest supposed to do, does it only > > handle the detection or it should handle the conversion too)? What will > be > > the consequence of not having such a transformer but the version is > > incompatible? > > > > Best, > > Yishun > > > > On Sat, Aug 11, 2018 at 11:27 AM Guozhang Wang <wangg...@gmail.com> > wrote: > > > > > 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 > > > > > > > > > -- > -- Guozhang >
