kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yishun Guan <gyis...@gmail.com>
Subject Re: [DISCUSS] KIP-347: Enable batching in FindCoordinatorRequest
Date Wed, 15 Aug 2018 23:44:48 GMT
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 <wangguoz@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 <gyishun@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 <wangguoz@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 <gyishun@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 <yuzhihong@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 <gyishun@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 <wangguoz@gmail.com> for his help
and
> > > patience!
> > > > > >
> > > > > > Thanks,
> > > > > > Yishun
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
>
>
> --
> -- Guozhang
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message