Hi David,

Thanks for the KIP. I know I'm late on voting here but I'd be +1 on this
too!

Just a quick clarification on the tag "name=Connections" of the third
metric, what would be the format of "Connections" here? Would that be the
connection listener name?


Guozhang


On Fri, Oct 11, 2019 at 1:49 PM Jun Rao <j...@confluent.io> wrote:

> Hi, David,
>
> Thanks for the KIP. Just a minor comment below.
>
> 100. It seems that the new flexible fields need tag numbers. Could you add
> them to the wiki?
>
> Jun
>
> On Mon, Sep 23, 2019 at 11:37 AM Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > Thanks David for the clarification. That sounds good.
> >
> > On Mon, Sep 23, 2019 at 12:35 AM David Jacot <dja...@confluent.io>
> wrote:
> >
> > > Hi all,
> > >
> > > The vote has passed with +3 binding votes (Colin McCabe, Gwen Shapira,
> > > Jason Gustafson) and +3 non binding votes (Mickael Maison, Konstantine
> > > Karantasis, Kevin Lu). \o/
> > >
> > > Thanks to everyone that reviewed and helped improve this proposal, and
> > > huge thanks to Colin for his great feedback.
> > >
> > > Best,
> > > David
> > >
> > > On Mon, Sep 23, 2019 at 9:28 AM David Jacot <dja...@confluent.io>
> wrote:
> > >
> > > > Hi Jason,
> > > >
> > > > The response will be a flexible version but the response header won't
> > be
> > > > (only for the api versions response). I have forgotten to change this
> > > point
> > > > in the KIP. I will make this point clearer.
> > > >
> > > > I didn't know that INVALID_REQUEST already exists. Yes, it makes
> sense
> > to
> > > > reuse it then.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Sat, Sep 21, 2019 at 3:02 AM Kevin Lu <lu.ke...@berkeley.edu>
> > wrote:
> > > >
> > > >> +1 (non-binding)
> > > >>
> > > >> Definitely needed this before as it would have saved me some time
> from
> > > >> trying to guess a client's version from api version/source code.
> > Thanks
> > > >> for
> > > >> the KIP!
> > > >>
> > > >> Regards,
> > > >> Kevin
> > > >>
> > > >> On Fri, Sep 20, 2019 at 4:29 PM Jason Gustafson <ja...@confluent.io
> >
> > > >> wrote:
> > > >>
> > > >> > +1 from me. This is a clever solution. Kind of a pity we couldn't
> > work
> > > >> > flexible version support into the response, but I understand why
> it
> > is
> > > >> > difficult.
> > > >> >
> > > >> > One minor nitpick: the INVALID_REQUEST error already exists. Are
> you
> > > >> > intending to reuse it?
> > > >> >
> > > >> > Thanks,
> > > >> > Jason
> > > >> >
> > > >> > On Fri, Sep 20, 2019 at 3:50 PM Konstantine Karantasis <
> > > >> > konstant...@confluent.io> wrote:
> > > >> >
> > > >> > > Quite useful KIP from an operational standpoint and I also like
> it
> > > in
> > > >> its
> > > >> > > most recent revised form.
> > > >> > >
> > > >> > > +1 (non-binding).
> > > >> > >
> > > >> > > Konstantine
> > > >> > >
> > > >> > >
> > > >> > > On Fri, Sep 20, 2019 at 9:55 AM Gwen Shapira <g...@confluent.io
> >
> > > >> wrote:
> > > >> > >
> > > >> > > > +1 (binding)
> > > >> > > >
> > > >> > > > Thanks for the KIP, David and for the help with the design,
> > > Colin. I
> > > >> > > > think it looks great now.
> > > >> > > >
> > > >> > > > On Fri, Sep 20, 2019 at 9:42 AM Colin McCabe <
> > cmcc...@apache.org>
> > > >> > wrote:
> > > >> > > > >
> > > >> > > > > On Fri, Sep 20, 2019, at 01:45, David Jacot wrote:
> > > >> > > > > > > Thanks for the clarification.  The proposed behavior
> > sounds
> > > >> > > > reasonable.
> > > >> > > > > > > Can you add a note about the implementation on the
> client?
> > > >> The
> > > >> > > > client
> > > >> > > > > > > needs to be prepared to handle > a response that doesn't
> > > >> include
> > > >> > > the
> > > >> > > > > > > versions, as well, since v1 did not.
> > > >> > > > > >
> > > >> > > > > > I have added a note about the implementation in the KIP.
> In
> > > >> short,
> > > >> > > > when the
> > > >> > > > > > client receives an unsupported version, it defaults to
> > version
> > > >> 0.
> > > >> > As
> > > >> > > > > > version 0 contains both the ErrorCode and the ApiKeys
> > fields,
> > > >> the
> > > >> > > > client
> > > >> > > > > > can check the error and in case of UNSUPPORTED_VERSION, it
> > can
> > > >> > check
> > > >> > > > the
> > > >> > > > > > ApiKeys to get the supported versions. If not present, it
> > > >> default
> > > >> > to
> > > >> > > > > > version 0.
> > > >> > > > > >
> > > >> > > > > > > Hmm. Like we discussed above, there is a very important
> > > >> > difference
> > > >> > > > in the
> > > >> > > > > > > v3 response, which is that the versions will be included
> > > even
> > > >> if
> > > >> > > the
> > > >> > > > > > > client's version was higher than what the broker
> supports.
> > > >> > > > > > > We should add a comment about that.
> > > >> > > > > >
> > > >> > > > > > Yeah. I think the change that we propose to enhance the
> > > >> handling of
> > > >> > > > > > unsupported versions of ApiVersionsRequest/Response is
> > > >> orthogonal
> > > >> > to
> > > >> > > > the
> > > >> > > > > > version bump. Concretely, the versions will be included in
> > the
> > > >> > > > > > ApiVersionsResponse v0 - the request/response used by the
> > > broker
> > > >> > when
> > > >> > > > > > failing back - so it is a bit misleading to say that
> > starting
> > > >> from
> > > >> > > > version
> > > >> > > > > > 3, the broker populate the ApiKeys field with the
> > information
> > > >> about
> > > >> > > the
> > > >> > > > > > supported versions of the AVR. I would rather put a note
> > > saying:
> > > >> > > > Starting
> > > >> > > > > > from Apache Kafka 2.4, ApiKeys field is populated with the
> > > >> > supported
> > > >> > > > > > versions of the ApiVersionsRequest when an
> > UNSUPPORTED_VERSION
> > > >> > error
> > > >> > > is
> > > >> > > > > > returned. Would this work for you?
> > > >> > > > >
> > > >> > > > > Thanks for the clarification.  Yes, that makes sense.
> Adding
> > > the
> > > >> > > > additional fields doesn't need to be tied to the version of
> > > >> > > > ApiVersionsResponse.
> > > >> > > > >
> > > >> > > > > Keep in mind, though, that you still have to handle
> responses
> > > from
> > > >> > > older
> > > >> > > > brokers, which will not include this information.  I assume
> that
> > > you
> > > >> > will
> > > >> > > > distinguish those responses based on the length of the
> response.
> > > We
> > > >> > > should
> > > >> > > > add this detail to the KIP.
> > > >> > > > >
> > > >> > > > > +1 (binding) after that change.
> > > >> > > > >
> > > >> > > > > cheers,
> > > >> > > > > Colin
> > > >> > > > >
> > > >> > > > > >
> > > >> > > > > > > Agreed.  This is a good use-case for INVALID_REQUEST.
> We
> > > >> should
> > > >> > > add
> > > >> > > > a
> > > >> > > > > > comment that this is now a valid error.
> > > >> > > > > >
> > > >> > > > > > I have documented the error in the Public Interfaces
> > section.
> > > >> > > > > >
> > > >> > > > > > Best,
> > > >> > > > > > David
> > > >> > > > > >
> > > >> > > > > > On Thu, Sep 19, 2019 at 10:52 PM Colin McCabe <
> > > >> cmcc...@apache.org>
> > > >> > > > wrote:
> > > >> > > > > >
> > > >> > > > > > > On Wed, Sep 18, 2019, at 23:44, David Jacot wrote:
> > > >> > > > > > > > Hi Colin,
> > > >> > > > > > > >
> > > >> > > > > > > > Thank you for your feedback! Please find my
> > > comments/answers
> > > >> > > below:
> > > >> > > > > > > >
> > > >> > > > > > > > *> Nitpick: in the intro paragraph, "Operators of
> Apache
> > > >> Kafka
> > > >> > > > clusters
> > > >> > > > > > > > have literally no information about the clients
> > connected
> > > to
> > > >> > > their
> > > >> > > > > > > > clusters" seems a bit too strong.  We have some
> > > information,
> > > >> > > > right?  For
> > > >> > > > > > > > example, the client ID, where clients are connecting
> > from,
> > > >> etc.
> > > >> > > > Maybe it
> > > >> > > > > > > > would be clearer to say "very little information about
> > the
> > > >> type
> > > >> > > of
> > > >> > > > client
> > > >> > > > > > > > software..."*
> > > >> > > > > > > >
> > > >> > > > > > > > That's fair. I will update it.
> > > >> > > > > > > >
> > > >> > > > > > > > *> Instead of ClientName and ClientVersion, how about
> > > >> > > > ClientSoftwareName
> > > >> > > > > > > > and ClientSoftwareVersion?  This might make it clearer
> > > what
> > > >> the
> > > >> > > > fields
> > > >> > > > > > > are
> > > >> > > > > > > > for.  I can see people getting confused about the
> > > difference
> > > >> > > > between
> > > >> > > > > > > > ClientName and ClientId, which sound pretty similar.
> > > Adding
> > > >> > > > "Software"
> > > >> > > > > > > to
> > > >> > > > > > > > the field name makes it much clearer what the
> difference
> > > is
> > > >> > > > between these
> > > >> > > > > > > > fields.*
> > > >> > > > > > > >
> > > >> > > > > > > > Good point. I like your suggestion. I will update it.
> > > >> > > > > > > >
> > > >> > > > > > > > *> In the "ApiVersions Request/Response Handling"
> > section,
> > > >> it
> > > >> > > > seems like
> > > >> > > > > > > > there is some out-of-date text.  Specifically, it says
> > "we
> > > >> > > propose
> > > >> > > > to add
> > > >> > > > > > > > the supported version of the ApiVersionsRequest in the
> > > >> response
> > > >> > > > sent back
> > > >> > > > > > > > to the client alongside the error...".  But on the
> other
> > > >> hand,
> > > >> > > > elsewhere
> > > >> > > > > > > in
> > > >> > > > > > > > the KIP, we say "ApiVersionsResponse is bumped to
> > version
> > > 3
> > > >> but
> > > >> > > > does not
> > > >> > > > > > > > have any changes in the schema"  Based on the offline
> > > >> > discussion
> > > >> > > > we had,
> > > >> > > > > > > > the correct text is the latter (we're not changing
> > > >> > > > ApiVersionsRerponse).
> > > >> > > > > > > > So we should remove the text about adding stuff to
> > > >> > > > ApiVersionsResponse.*
> > > >> > > > > > > >
> > > >> > > > > > > > Sorry for the confusion. I think my usage of the word
> > > "add"
> > > >> is
> > > >> > > not
> > > >> > > > > > > > appropriate here. The ApiVersionsResponse won't change
> > as
> > > >> you
> > > >> > > > said. My
> > > >> > > > > > > > proposal is to provide the supported versions of the
> > > >> > > > ApiVersionsRequest
> > > >> > > > > > > in
> > > >> > > > > > > > the response by populating the existing `api_versions`
> > > >> field.
> > > >> > In
> > > >> > > > the
> > > >> > > > > > > > current version, when an error occurs, the
> > `api_versions`
> > > is
> > > >> > > empty
> > > >> > > > in the
> > > >> > > > > > > > response. Providing it enables the client to re-send
> the
> > > >> latest
> > > >> > > > version
> > > >> > > > > > > > supported by the broker instead of defaulting to
> zero. I
> > > >> will
> > > >> > > > update the
> > > >> > > > > > > > KIP to make this clearer.
> > > >> > > > > > >
> > > >> > > > > > > Thanks for the clarification.  The proposed behavior
> > sounds
> > > >> > > > reasonable.
> > > >> > > > > > > Can you add a note about the implementation on the
> client?
> > > >> The
> > > >> > > > client
> > > >> > > > > > > needs to be prepared to handle a response that doesn't
> > > include
> > > >> > the
> > > >> > > > > > > versions, as well, since v1 did not.
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > *> In a similar vein, the comment "  // Version 3 is
> > > >> similar to
> > > >> > > > version
> > > >> > > > > > > 2"
> > > >> > > > > > > > should be " // Version 3 is identical to version 2" or
> > > >> > something
> > > >> > > > like
> > > >> > > > > > > > that.  Although I guess technically things which are
> > > >> identical
> > > >> > > are
> > > >> > > > also
> > > >> > > > > > > > similar, the current phrasing could be misleading.*
> > > >> > > > > > > >
> > > >> > > > > > > > Good point. I will use `Version 3 is the same as
> version
> > > 2.`
> > > >> > > which
> > > >> > > > is the
> > > >> > > > > > > > statement already used in other requests/responses.
> > > >> > > > > > >
> > > >> > > > > > > Hmm. Like we discussed above, there is a very important
> > > >> > difference
> > > >> > > > in the
> > > >> > > > > > > v3 response, which is that the versions will be included
> > > even
> > > >> if
> > > >> > > the
> > > >> > > > > > > client's version was higher than what the broker
> supports.
> > > We
> > > >> > > > should add a
> > > >> > > > > > > comment about that.
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > *> Now that KIP-482 has been accepted, I think there
> > are a
> > > >> few
> > > >> > > > things
> > > >> > > > > > > that
> > > >> > > > > > > > are worth clarifying in the KIP.  Firstly,
> > > >> ApiVersionsRequest
> > > >> > v3
> > > >> > > > should
> > > >> > > > > > > be
> > > >> > > > > > > > a "flexible version".  Mainly, that means its request
> > > header
> > > >> > will
> > > >> > > > support
> > > >> > > > > > > > optional tagged fields.  However, ApiVersionsResponse
> v3
> > > >> will
> > > >> > > *not*
> > > >> > > > > > > support
> > > >> > > > > > > > optional tagged fields in its response header.  This
> is
> > > >> > necessary
> > > >> > > > > > > because--
> > > >> > > > > > > > as you said-- the broker must look at a fixed offset
> to
> > > find
> > > >> > the
> > > >> > > > error
> > > >> > > > > > > > code, regardless of the response version.*
> > > >> > > > > > > > Right. I have put it because I thought your PR would
> do
> > > it.
> > > >> I
> > > >> > > will
> > > >> > > > update
> > > >> > > > > > > > this. By the way, it means that the request/response
> > must
> > > be
> > > >> > > > updated to
> > > >> > > > > > > the
> > > >> > > > > > > > generated ones, isn't it? AVR is still using the old
> > > >> mechanism.
> > > >> > > > > > >
> > > >> > > > > > > Yeah, I think we should move to the new mechanism.  It
> > > should
> > > >> be
> > > >> > > > very easy
> > > >> > > > > > > for the request.  The response may be slightly more
> > > difficult,
> > > >> > but
> > > >> > > > probably
> > > >> > > > > > > not that much more.
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > *> I think we should force client software names and
> > > >> versions
> > > >> > to
> > > >> > > > follow a
> > > >> > > > > > > > regular expression and disconnect if they do not.
> This
> > > will
> > > >> > > > prevent
> > > >> > > > > > > issues
> > > >> > > > > > > > when using these strings in metrics names.  Probably
> we
> > > want
> > > >> > > > something
> > > >> > > > > > > > like:> [\.\-A-Za-z0-9]?[\.\-A-Za-z0-9
> > ]*[\.\-A-Za-z0-9]?>
> > > >> > Notice
> > > >> > > > this
> > > >> > > > > > > does
> > > >> > > > > > > > _not* include underscores, since they get converted to
> > > dots
> > > >> in
> > > >> > > JMX,
> > > >> > > > > > > causing
> > > >> > > > > > > > ambiguity.  It also doesn't allow the first or last
> > > >> character
> > > >> > to
> > > >> > > > be a
> > > >> > > > > > > > space.*
> > > >> > > > > > > >
> > > >> > > > > > > > I do agree and I have already put something along
> those
> > > >> lines
> > > >> > in
> > > >> > > > the
> > > >> > > > > > > > proposal. See the "Validation" chapter. I have
> proposed
> > to
> > > >> use
> > > >> > a
> > > >> > > > more
> > > >> > > > > > > > restrictive validation which does not allow white
> > spaces.
> > > I
> > > >> > think
> > > >> > > > spaces
> > > >> > > > > > > > wouldn't be used in software name nor version. Is it
> OK
> > > for
> > > >> you
> > > >> > > if
> > > >> > > > we
> > > >> > > > > > > stick
> > > >> > > > > > > > to the more restrictive one? Thank your letting me
> know
> > > >> about
> > > >> > the
> > > >> > > > > > > > underscores. I have missed this.
> > > >> > > > > > >
> > > >> > > > > > > Yeah, the one you proposed sounds fine.
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Regarding disconnecting when the validation fails,
> this
> > is
> > > >> > what I
> > > >> > > > have
> > > >> > > > > > > > proposed as well. Magnus has brought a good point
> > though.
> > > >> Using
> > > >> > > an
> > > >> > > > > > > explicit
> > > >> > > > > > > > error like `INVALID_REQUEST` may be better. In this
> > case,
> > > >> the
> > > >> > > > client
> > > >> > > > > > > would
> > > >> > > > > > > > have to disconnect when it happens. I will update the
> > KIP
> > > to
> > > >> > > > reflect
> > > >> > > > > > > this.
> > > >> > > > > > >
> > > >> > > > > > > Agreed.  This is a good use-case for INVALID_REQUEST.
> We
> > > >> should
> > > >> > > add
> > > >> > > > a
> > > >> > > > > > > comment that this is now a valid error.
> > > >> > > > > > >
> > > >> > > > > > > best,
> > > >> > > > > > > Colin
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Best,
> > > >> > > > > > > > David
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > On Wed, Sep 18, 2019 at 9:46 PM Colin McCabe <
> > > >> > cmcc...@apache.org
> > > >> > > >
> > > >> > > > wrote:
> > > >> > > > > > > >
> > > >> > > > > > > > > That's fair.  We could use the existing error code
> in
> > > the
> > > >> > > > response and
> > > >> > > > > > > > > pass back something like INVALID_REQUEST.
> > > >> > > > > > > > >
> > > >> > > > > > > > > I'm not sure if we want to add an error string field
> > > just
> > > >> for
> > > >> > > > this
> > > >> > > > > > > > > (although they're a good idea in general...)
> > > >> > > > > > > > >
> > > >> > > > > > > > > best,
> > > >> > > > > > > > > Colin
> > > >> > > > > > > > >
> > > >> > > > > > > > > On Wed, Sep 18, 2019, at 12:31, Magnus Edenhill
> wrote:
> > > >> > > > > > > > > > > I think we should force client software names
> and
> > > >> > versions
> > > >> > > to
> > > >> > > > > > > follow a
> > > >> > > > > > > > > > regular expression and disconnect if they do not.
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > Disconnecting is not really a great error
> > propagation
> > > >> > method
> > > >> > > > since it
> > > >> > > > > > > > > > leaves the client oblivious to what went wrong.
> > > >> > > > > > > > > > Instead suggest we return an ApiVersionResponse
> with
> > > an
> > > >> > error
> > > >> > > > code
> > > >> > > > > > > and a
> > > >> > > > > > > > > > human-readable error message field.
> > > >> > > > > > > > > >
> > > >> > > > > > > > > >
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > Den ons 18 sep. 2019 kl 20:05 skrev Colin McCabe <
> > > >> > > > cmcc...@apache.org
> > > >> > > > > > > >:
> > > >> > > > > > > > > >
> > > >> > > > > > > > > > > Hi David,
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Thanks for the KIP!
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Nitpick: in the intro paragraph, "Operators of
> > > Apache
> > > >> > Kafka
> > > >> > > > > > > clusters
> > > >> > > > > > > > > have
> > > >> > > > > > > > > > > literally no information about the clients
> > connected
> > > >> to
> > > >> > > their
> > > >> > > > > > > clusters"
> > > >> > > > > > > > > > > seems a bit too strong.  We have some
> information,
> > > >> right?
> > > >> > > > For
> > > >> > > > > > > > > example, the
> > > >> > > > > > > > > > > client ID, where clients are connecting from,
> etc.
> > > >> Maybe
> > > >> > > it
> > > >> > > > would
> > > >> > > > > > > be
> > > >> > > > > > > > > > > clearer to say "very little information about
> the
> > > >> type of
> > > >> > > > client
> > > >> > > > > > > > > > > software..."
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Instead of ClientName and ClientVersion, how
> about
> > > >> > > > > > > ClientSoftwareName
> > > >> > > > > > > > > and
> > > >> > > > > > > > > > > ClientSoftwareVersion?  This might make it
> clearer
> > > >> what
> > > >> > the
> > > >> > > > fields
> > > >> > > > > > > are
> > > >> > > > > > > > > > > for.  I can see people getting confused about
> the
> > > >> > > difference
> > > >> > > > > > > between
> > > >> > > > > > > > > > > ClientName and ClientId, which sound pretty
> > similar.
> > > >> > > Adding
> > > >> > > > > > > > > "Software" to
> > > >> > > > > > > > > > > the field name makes it much clearer what the
> > > >> difference
> > > >> > is
> > > >> > > > between
> > > >> > > > > > > > > these
> > > >> > > > > > > > > > > fields.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > In the "ApiVersions Request/Response Handling"
> > > >> section,
> > > >> > it
> > > >> > > > seems
> > > >> > > > > > > like
> > > >> > > > > > > > > > > there is some out-of-date text.  Specifically,
> it
> > > says
> > > >> > "we
> > > >> > > > propose
> > > >> > > > > > > to
> > > >> > > > > > > > > add
> > > >> > > > > > > > > > > the supported version of the ApiVersionsRequest
> in
> > > the
> > > >> > > > response
> > > >> > > > > > > sent
> > > >> > > > > > > > > back
> > > >> > > > > > > > > > > to the client alongside the error...".  But on
> the
> > > >> other
> > > >> > > > hand,
> > > >> > > > > > > > > elsewhere in
> > > >> > > > > > > > > > > the KIP, we say "ApiVersionsResponse is bumped
> to
> > > >> > version 3
> > > >> > > > but
> > > >> > > > > > > does
> > > >> > > > > > > > > not
> > > >> > > > > > > > > > > have any changes in the schema"  Based on the
> > > offline
> > > >> > > > discussion we
> > > >> > > > > > > > > had,
> > > >> > > > > > > > > > > the correct text is the latter (we're not
> changing
> > > >> > > > > > > > > ApiVersionsRerponse).
> > > >> > > > > > > > > > > So we should remove the text about adding stuff
> to
> > > >> > > > > > > ApiVersionsResponse.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > In a similar vein, the comment "  // Version 3
> is
> > > >> similar
> > > >> > > to
> > > >> > > > > > > version 2"
> > > >> > > > > > > > > > > should be " // Version 3 is identical to version
> > 2"
> > > or
> > > >> > > > something
> > > >> > > > > > > like
> > > >> > > > > > > > > > > that.  Although I guess technically things which
> > are
> > > >> > > > identical are
> > > >> > > > > > > also
> > > >> > > > > > > > > > > similar, the current phrasing could be
> misleading.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Now that KIP-482 has been accepted, I think
> there
> > > are
> > > >> a
> > > >> > few
> > > >> > > > things
> > > >> > > > > > > that
> > > >> > > > > > > > > > > are worth clarifying in the KIP.  Firstly,
> > > >> > > > ApiVersionsRequest v3
> > > >> > > > > > > > > should be
> > > >> > > > > > > > > > > a "flexible version".  Mainly, that means its
> > > request
> > > >> > > header
> > > >> > > > will
> > > >> > > > > > > > > support
> > > >> > > > > > > > > > > optional tagged fields.  However,
> > > ApiVersionsResponse
> > > >> v3
> > > >> > > > will *not*
> > > >> > > > > > > > > support
> > > >> > > > > > > > > > > optional tagged fields in its response header.
> > This
> > > >> is
> > > >> > > > necessary
> > > >> > > > > > > > > because--
> > > >> > > > > > > > > > > as you said-- the broker must look at a fixed
> > offset
> > > >> to
> > > >> > > find
> > > >> > > > the
> > > >> > > > > > > error
> > > >> > > > > > > > > > > code, regardless of the response version.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > I think we should force client software names
> and
> > > >> > versions
> > > >> > > to
> > > >> > > > > > > follow a
> > > >> > > > > > > > > > > regular expression and disconnect if they do
> not.
> > > >> This
> > > >> > > will
> > > >> > > > > > > prevent
> > > >> > > > > > > > > issues
> > > >> > > > > > > > > > > when using these strings in metrics names.
> > Probably
> > > >> we
> > > >> > > want
> > > >> > > > > > > something
> > > >> > > > > > > > > like:
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > [\.\-A-Za-z0-9]?[\.\-A-Za-z0-9
> ]*[\.\-A-Za-z0-9]?
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > Notice this does _not* include underscores,
> since
> > > they
> > > >> > get
> > > >> > > > > > > converted to
> > > >> > > > > > > > > > > dots in JMX, causing ambiguity.  It also doesn't
> > > allow
> > > >> > the
> > > >> > > > first or
> > > >> > > > > > > > > last
> > > >> > > > > > > > > > > character to be a space.
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > best,
> > > >> > > > > > > > > > > Colin
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > > > On Mon, Sep 16, 2019, at 04:39, Mickael Maison
> > > wrote:
> > > >> > > > > > > > > > > > +1 (non binding)
> > > >> > > > > > > > > > > > Thanks for the KIP!
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > > > On Mon, Sep 16, 2019 at 12:07 PM David Jacot <
> > > >> > > > > > > dja...@confluent.io>
> > > >> > > > > > > > > > > wrote:
> > > >> > > > > > > > > > > > >
> > > >> > > > > > > > > > > > > Hi all,
> > > >> > > > > > > > > > > > >
> > > >> > > > > > > > > > > > > I would like to start a vote on KIP-511:
> > > >> > > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > >
> > > >> > > > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers
> > > >> > > > > > > > > > > > >
> > > >> > > > > > > > > > > > > Best,
> > > >> > > > > > > > > > > > > David
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > >
> > > >> > > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > >
> >
>


-- 
-- Guozhang

Reply via email to