I see! Thanks! On Thu, Aug 16, 2018, 4:35 PM Guozhang Wang <wangg...@gmail.com> wrote:
> It is not implemented, but should not be hard to do so (and again you do > NOT have to do that in this KIP, I'm bringing this up so that you can help > thinking about the process). > > Quoting from Colin's comment: > > " > The pattern is that you would try to send a request for more than one > group, and then you would get an UnsupportedVersionException (nothing would > be sent on the wire, this is purely internal to the code). > Then your code would handle the UVE by creating requests with an older > version that only had one group each. > " > > > Guozhang > > > On Wed, Aug 15, 2018 at 4:44 PM, Yishun Guan <gyis...@gmail.com> wrote: > > > 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 > > > > > > > > > -- > -- Guozhang >
