You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Ashish Singh <as...@cloudera.com> on 2016/04/01 00:05:37 UTC
Re: [DISCUSS] KIP-35 - Retrieve protocol version
I agree with Jason. The blocking factor has been how to use the proposed
changes in java client without making it super complicated. With Jason's
suggestion we can get past this blocker, while keeping the core of the
proposal intact.
On Thu, Mar 31, 2016 at 2:51 PM, Jason Gustafson <ja...@confluent.io> wrote:
> Bumping this thread. I talked with Ashish and Magnus about this KIP offline
> and I'm gradually coming over. The new API actually stands by itself
> outside of the discussion about whether the client should support backwards
> compatibility or not. For the Java client, we could continue to support the
> current compatibility approach in which the client supports only brokers
> with the same version or greater. In that case, we would use this API only
> to assert that the current API versions are all supported, and raise an
> exception if they are not. This gives us the capability going forward to
> detect when the client is talking to an older broker, which we don't have
> right now. This check should be straightforward, so we could do it now,
> which would resolve some of the uneasiness about having an unused feature
> which we depended on other clients to test for us. Does that make sense or
> not?
>
> -Jason
>
> On Thu, Mar 17, 2016 at 4:06 PM, Ashish Singh <as...@cloudera.com> wrote:
>
> > We have proposed and discussed majorly three approaches so far, there
> were
> > many minor versions with small variations. Comparing them really
> requires a
> > side by side proposal and their pros/cons, and I agree with others that
> > this has been lacking in the KIP. We just updated the KIP with following
> > details.
> >
> > 1. Provide proposed changes in all the three proposals we have discussed
> so
> > far. Except the current proposal, these proposals are in rejected
> > alternatives.
> > 2. Provide reasoning on why the rejected proposals were rejected.
> > 3. Add scenarios for all of these proposals from a client developer and
> > core Kafka developer point of view.
> >
> > As we are really close to 0.10 deadline, a quick round of voting will
> > really help. If you really do not like the idea, please feel free to say
> > so. If the vote fails for the current proposal, it can at lease provide
> > recommendations that we should consider for next version of proposal and
> > put it up for vote again for next release. However, as stated earlier by
> > multiple people having this ASAP will be awesome.
> >
> > On Thu, Mar 17, 2016 at 3:29 PM, Dana Powers <da...@gmail.com>
> > wrote:
> >
> > > On Thu, Mar 17, 2016 at 1:42 PM, Gwen Shapira <gw...@confluent.io>
> wrote:
> > >
> > > > "I think I would make this approach work by looking at the released
> > > server
> > > > version documentation for each version that I am trying to support
> and
> > > test
> > > > against*, manually identify the expected "protocol vectors" each
> > > supports,
> > > > store that as a map of vectors to "broker versions", check each
> vector
> > at
> > > > runtime until I find a match, and write code compatibility checks
> from
> > > > there."
> > > >
> > > > How is this better than a global version ID?
> > >
> > >
> > > As a client developer, it seems roughly the same. I think it probably
> > > avoids the server development workflow issues, and possibly the need to
> > > agree on semantics of the global version ID? But others surely are more
> > > qualified than me to comment on that part.
> > >
> > > -Dana
> > >
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
> >
>
--
Regards,
Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ashish Singh <as...@cloudera.com>.
Ismael, thanks for the review.
On Fri, Apr 1, 2016 at 1:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
> A couple of questions:
>
> 1. The KIP says "Specific version may be deprecated through protocol
> documentation but must still be supported (although it is fair to return an
> error code if the specific API supports it).". It may be worth expanding
> this a little more. For example, what does it mean to support the API? I
> guess this means that the broker must not disconnect the client and the
> broker must return a valid protocol response.
Sure, will add to KIP.
> Given that it says that it is
> "fair" (I would probably replace "fair" with "valid") to return an error
> code if the specific API supports it, it sounds like we are saying that we
> don't have to maintain the semantic behaviour (i.e. we could _always_
> return an error for a deprecated API?). Is this true?
>
Yes. Typically we should support deprecated APIs, however in cases where an
intermediate buggy version was backported, if the api has error field, it
should be valid to use it to signal an error. Example of such backporting
is provided on KIP, below is excerpt. Note that this KIP is not asking for
any new error type.
> For instance, say 0.9.0 had protocol versions [0] for api key 1. On
trunk, version 1 of the api key was added. Users running off trunk started
using version 1 of the api and found out a major bug. To rectify that
version 2 of the api is added to trunk. For some reason, it is now deemed
important to have version 2 of the api in 0.9.1 as well. To do so, version
1 and version 2 both of the api will be backported to the 0.9.1 branch.
0.9.1 broker will return 0 as min supported version for the api and 2 for
the max supported version for the api. However, the version 1 should be
clearly marked as deprecated on its documentation. It will be client's
responsibility to make sure they are not using any such deprecated version
to the best knowledge of the client at the time of development (or
alternatively by configuration).
>
> 2. ApiVersionQueryRequest seems a bit verbose, why not ApiVersionRequest?
>
Adopted.
>
> Ismael
>
--
Regards,
Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Rajini,
Yes, I agree that it's not ideal. However, doing this once at the broker is
more manageable than pushing the additional complexity to the clients.
Between the two options you outlined, the second one seemed the least bad
(we do something similar for controlled shutdown because it was missing a
header before 0.9.0.0).
Ismael
On Tue, Apr 12, 2016 at 9:56 AM, Rajini Sivaram <
rajinisivaram@googlemail.com> wrote:
> Ismael,
>
> My only concern about wrapping SASL tokens in Kafka headers is backward
> compatibility. We would either have a different format for GSSAPI alone to
> match 0.9.0.x or we would need to support two different wire protocols for
> GSSAPI. Neither sounds ideal.
>
> On Tue, Apr 12, 2016 at 9:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Jun,
> >
> > I understand the point about the SASL tokens being similar to the SSL
> > handshake in a way. However, is there any SASL library that handles the
> > network communication for these tokens? I couldn't find any and without
> > that, there isn't much benefit in deviating from Kafka's protocol (we
> > basically save the space taken by the request header). It's worth
> > mentioning that we are already adding the message size before the opaque
> > bytes provided by the library, so one could say we are already extending
> > the protocol.
> >
> > If we leave versioning aside, adding the standard Kafka request header to
> > those messages may also help from a debugging perspective as would then
> > include client id and correlation id along with the message.
> >
> > Ismael
> >
> > On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Magnus,
> > >
> > > That sounds reasonable. To reduce the changes on the server side, I'd
> > > suggest the following minor tweaks on the proposal.
> > >
> > > 1. Continue supporting the separate SASL and SASL_SSL port.
> > >
> > > On SASL port, we support the new sequence
> > > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> > > regular
> > > requests
> > >
> > > On SASL_SSL port, we support the new sequence
> > > SSL handshake bytes, ApiVersionRequest (optional),
> > > SaslHandshakeRequest,
> > > SASL tokens, regular requests
> > >
> > > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your
> argument
> > > about SSL handshake, those SASL tokens are generated by SASL library
> and
> > > Kafka doesn't really control its versioning. Kafka only controls the
> > > selection of SASL mechanism, which will be versioned in
> > > SaslHandshakeRequest.
> > >
> > > Does that work for you?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > >
> > > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> > > wrote:
> > >
> > > > Hey Jun, see inline
> > > >
> > > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > >
> > > > > Hi, Magnus,
> > > > >
> > > > > Let me understand your proposal in more details just from the
> > client's
> > > > > perspective. My understanding of your proposal is the following.
> > > > >
> > > > > On plaintext port, the client will send the following bytes in
> order.
> > > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL
> is
> > > > > enabled), regular requests
> > > > >
> > > > > On SSL port, the client will send the following bytes in order.
> > > > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest,
> > SASL
> > > > > tokens (if SASL is enabled), regular requests
> > > > >
> > > >
> > > >
> > > > Yup!
> > > > "SASL tokens" is a series of proper Kafka protocol
> > SaslHandshakeRequests
> > > > until
> > > > the handshake is done.
> > > >
> > > >
> > > >
> > > > >
> > > > > Is that right? Since we can use either SSL or SASL for
> > authentication,
> > > > it's
> > > > > weird that in one case, we require ApiVersionRequest to happen
> before
> > > > > authentication and in another case we require the reverse.
> > > > >
> > > >
> > > > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > > > libraries it
> > > > doesnt make sense to reimplement that on top of Kafka, so it isn't
> > really
> > > > comparable.
> > > > But for SASL there is no standardised handshake protocol so we must
> > > either
> > > > conceive one from scratch, or use the protocol that we already have
> > > > (Kafka).
> > > > For the initial SASL implementation in 0.9 the first option was
> chosen
> > > and
> > > > while
> > > > it required a new protocol implementation in supporting clients and
> the
> > > > broker
> > > > it served its purpose. But not for long, it already needs to evolve,
> > > > and this gives us a golden[1] opportunity to make the implementation
> > > > reusable, evolvable, less complex
> > > > and in line with all our other protocol work, by using the protocol
> > > stack
> > > > of Kafka which the
> > > > broker and all clients already have in place.
> > > >
> > > > Not taking this chance and instead diverging the custom SASL
> handshake
> > > > protocol
> > > > even further from Kafka seems to me a weird choice.
> > > >
> > > > The current KIP-43 proposal does not have a clear compatibility
> story;
> > it
> > > > doesnt seem to be possible
> > > > to upgrade clients before brokers, while this might be okay for the
> > Java
> > > > client, the KIP-35 discussion
> > > > has hopefully proven that this assumption can't be made for the
> entire
> > > > eco-system.
> > > >
> > > > Let me be clear that there isn't anything technically wrong with the
> > > KIP-43
> > > > proposal (well,
> > > > except for the hack to check byte[0] for 0x60 perhaps), but I'm
> worried
> > > the
> > > > proposal will eventually lead to
> > > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
> > handshake,
> > > > and this is just redundant,
> > > > there is no technical reason for doing so and it'll just make
> protocol
> > > > semantics and implementations more complex.
> > > >
> > > >
> > > > Regards,
> > > > Magnus
> > > >
> > > > [1]: Timing is good for this change since only two clients, Java and
> C,
> > > > currently supports
> > > > the existing SASL handshake so far.
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
> > magnus@edenhill.se>
> > > > > wrote:
> > > > >
> > > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > > > >
> > > > > > > Thinking about ApiVersionRequest a bit more. There are quite a
> > few
> > > > > things
> > > > > > > special about it. In the ideal case, (1) its version should
> never
> > > > > change;
> > > > > > >
> > > > > >
> > > > > > The only thing we know of the future is that we dont know
> anything,
> > > we
> > > > > > can't
> > > > > > think of every possible future use case, that's why need to be
> able
> > > to
> > > > > > evolve interfaces
> > > > > > as requirements and use-cases change. This is the gist of KIP-35,
> > and
> > > > > > hampering
> > > > > > KIP-35 itself, by not letting it also evolve, does not seem right
> > to
> > > me
> > > > > at
> > > > > > all.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > (2) it needs to be done before authentication (either
> SSL/SASL);
> > > (3)
> > > > it
> > > > > > is
> > > > > > > required to be issued at the beginning of each connection but
> > never
> > > > > needs
> > > > > > > to be issued again on the same connection. So, instead of
> > modeling
> > > it
> > > > > as
> > > > > > a
> > > > > > > regular request, it seems a cleaner approach is to just bake
> that
> > > > into
> > > > > > the
> > > > > > > initial connection handshake even before the authentication
> > layer.
> > > So
> > > > > the
> > > > > > > sequencing in a connection will be api discovery,
> authentication,
> > > > > > followed
> > > > > > > by regular requests. I am not sure we can still add this in a
> > > > backward
> > > > > > > compatible way now (e.g., not sure what the initial bytes from
> an
> > > SSL
> > > > > > > connection will look like). Even if we can do this in a
> backward
> > > > > > compatible
> > > > > > > way, it's probably non-trivial amount of work though.
> > > > > > >
> > > > > >
> > > > > > I have the luxory of not knowing the broker internals, so I can
> > only
> > > > > > discuss
> > > > > > this on a conceptual design level.
> > > > > >
> > > > > > In its simplest form each API request type has a NeedsAuth flag
> and
> > > the
> > > > > > broker protocol request layer simply checks if the current
> session
> > is
> > > > > > Authenticated
> > > > > > before processing the request: If not the session is closed and
> an
> > > > error
> > > > > is
> > > > > > logged.
> > > > > > The only two API requests that dont have the NeedsAuth flag would
> > be
> > > > > > SaslHandshakeRequest
> > > > > > and ApiVersionRequest, the latter could also use filtering to
> only
> > > > return
> > > > > > the same two
> > > > > > requests in ApiVersionResponse before the client is authenticated
> > (as
> > > > not
> > > > > > to "leak" information).
> > > > > > If authentication is not configured on the broker all sessions
> are
> > > > deemed
> > > > > > authenticated by default.
> > > > > >
> > > > > >
> > > > > > Re backwards compatibility:
> > > > > > My suggestion is to keep the current special SASL handshake
> > protocol
> > > on
> > > > > the
> > > > > > SASL_PLAIN/SASL_SSL
> > > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API
> > on
> > > > the
> > > > > > PLAIN/SSL endpoints.
> > > > > > This way the broker is backwards compatible with older clients
> that
> > > > only
> > > > > > supports the special SASL protocol,
> > > > > > and newer cliets are also backwards compatible with older brokers
> > > that
> > > > > only
> > > > > > supports the special SASL protocol.
> > > > > > Newer clients connecting to new brokers will be configured to use
> > > > > non-SASL
> > > > > > ports and use the
> > > > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > > > >
> > > > > > Using the existing standard Kafka protocol and the new
> future-proof
> > > > > > functionality of ApiVersionRequest
> > > > > > allows the in-band authentication mechanisms and semantics to
> > > naturally
> > > > > > evolve over time
> > > > > > without breaking existing clients.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > We started KIP-35 with supporting a client to know if a version
> > is
> > > > > > > supported by the broker. It's now evolved into supporting a
> > client
> > > to
> > > > > > > implement multiple versions of the protocol and dynamically
> pick
> > a
> > > > > > version
> > > > > > > supported by the broker. The former is likely solvable without
> > > > > > > ApiVersionRequest. How important is the latter? What if the C
> > > client
> > > > > just
> > > > > > > follows the java client model by implementing one version of
> > > protocol
> > > > > > per C
> > > > > > > client release (which seems easier to implement)?
> > > > > > >
> > > > > >
> > > > > > We've discussed this at length and it is not an option for
> > > librdkafka,
> > > > > nor
> > > > > > kafka-python, and
> > > > > > probably other clients as well, due to usability/UX and
> maintenance
> > > > > > reasons.
> > > > > > (There's even discussion of making the Java client more version
> > > > > agnostic!)
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
> > wrote:
> > > > > > >
> > > > > > > > Magnus,
> > > > > > > >
> > > > > > > > A while back, we had another proposal for the broker to just
> > send
> > > > the
> > > > > > > > correlation id and an empty payload if it receives an
> > unsupported
> > > > > > version
> > > > > > > > of the request. I didn't see that in the rejected section. It
> > > seems
> > > > > > > simpler
> > > > > > > > than the current proposal where the client has to issue an
> > > > > > > > ApiVersionRequest on every connection. Could you document the
> > > > reason
> > > > > > why
> > > > > > > we
> > > > > > > > rejected it?
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> > > asingh@cloudera.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > > >>
> > > > > > > >> > Two more things:
> > > > > > > >> >
> > > > > > > >> > 3. We talk about backporting of new request versions to
> > stable
> > > > > > > branches
> > > > > > > >> in
> > > > > > > >> > the KIP. In practice, we can't do that until the Java
> client
> > > is
> > > > > > > changed
> > > > > > > >> so
> > > > > > > >> > that it doesn't blindly use the latest protocol version.
> > > > > Otherwise,
> > > > > > if
> > > > > > > >> new
> > > > > > > >> > request versions were added to 0.9.0.2, the client would
> > break
> > > > > when
> > > > > > > >> talking
> > > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would
> fail a
> > > bit
> > > > > > more
> > > > > > > >> > gracefully, but that's still not good enough for a stable
> > > > branch).
> > > > > > It
> > > > > > > >> may
> > > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > > > orthogonal
> > > > > > and
> > > > > > > >> > doesn't prevent the KIP from being adopted, but good to
> > avoid
> > > > > > > >> confusion).
> > > > > > > >> >
> > > > > > > >> Good point. Adding this note and also adding a note that
> Kafka
> > > has
> > > > > not
> > > > > > > >> backported an api version so far.
> > > > > > > >>
> > > > > > > >> >
> > > > > > > >> > 4. The paragraph below is a bit confusing. It starts
> talking
> > > > about
> > > > > > > 0.9.0
> > > > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > > > >> >
> > > > > > > >> Yes.
> > > > > > > >>
> > > > > > > >> >
> > > > > > > >> > "Deprecation of a protocol version will be done by
> marking a
> > > > > > protocol
> > > > > > > >> > version as deprecated in protocol documentation.
> > Documentation
> > > > > shall
> > > > > > > >> also
> > > > > > > >> > be used to indicate a protocol version that must not be
> > used,
> > > or
> > > > > for
> > > > > > > any
> > > > > > > >> > such information.For instance, say 0.9.0 had protocol
> > versions
> > > > [0]
> > > > > > for
> > > > > > > >> api
> > > > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > > > running
> > > > > > off
> > > > > > > >> > trunk started using version 1 of the api and found out a
> > major
> > > > > bug.
> > > > > > To
> > > > > > > >> > rectify that version 2 of the api is added to trunk. For
> > some
> > > > > > reason,
> > > > > > > >> it is
> > > > > > > >> > now deemed important to have version 2 of the api in 0.9.1
> > as
> > > > > well.
> > > > > > To
> > > > > > > >> do
> > > > > > > >> > so, version 1 and version 2 both of the api will be
> > backported
> > > > to
> > > > > > the
> > > > > > > >> 0.9.1
> > > > > > > >> > branch. 0.9.1 broker will return 0 as min supported
> version
> > > for
> > > > > the
> > > > > > > api
> > > > > > > >> and
> > > > > > > >> > 2 for the max supported version for the api. However, the
> > > > version
> > > > > 1
> > > > > > > >> should
> > > > > > > >> > be clearly marked as deprecated on its documentation. It
> > will
> > > be
> > > > > > > >> client's
> > > > > > > >> > responsibility to make sure they are not using any such
> > > > deprecated
> > > > > > > >> version
> > > > > > > >> > to the best knowledge of the client at the time of
> > development
> > > > (or
> > > > > > > >> > alternatively by configuration)."
> > > > > > > >> >
> > > > > > > >> > Ismael
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> > > ismael@juma.me.uk>
> > > > > > > wrote:
> > > > > > > >> >
> > > > > > > >> > > A couple of questions:
> > > > > > > >> > >
> > > > > > > >> > > 1. The KIP says "Specific version may be deprecated
> > through
> > > > > > protocol
> > > > > > > >> > > documentation but must still be supported (although it
> is
> > > fair
> > > > > to
> > > > > > > >> return
> > > > > > > >> > an
> > > > > > > >> > > error code if the specific API supports it).". It may be
> > > worth
> > > > > > > >> expanding
> > > > > > > >> > > this a little more. For example, what does it mean to
> > > support
> > > > > the
> > > > > > > >> API? I
> > > > > > > >> > > guess this means that the broker must not disconnect the
> > > > client
> > > > > > and
> > > > > > > >> the
> > > > > > > >> > > broker must return a valid protocol response. Given that
> > it
> > > > says
> > > > > > > that
> > > > > > > >> it
> > > > > > > >> > is
> > > > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > > > return
> > > > > an
> > > > > > > >> error
> > > > > > > >> > > code if the specific API supports it, it sounds like we
> > are
> > > > > saying
> > > > > > > >> that
> > > > > > > >> > we
> > > > > > > >> > > don't have to maintain the semantic behaviour (i.e. we
> > could
> > > > > > > _always_
> > > > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > > > >> > >
> > > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > > > >> ApiVersionRequest?
> > > > > > > >> > >
> > > > > > > >> > > Ismael
> > > > > > > >> > >
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> --
> > > > > > > >>
> > > > > > > >> Regards,
> > > > > > > >> Ashish
> > > > > > > >>
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
>
> --
> Regards,
>
> Rajini
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Rajini Sivaram <ra...@googlemail.com>.
Ismael,
My only concern about wrapping SASL tokens in Kafka headers is backward
compatibility. We would either have a different format for GSSAPI alone to
match 0.9.0.x or we would need to support two different wire protocols for
GSSAPI. Neither sounds ideal.
On Tue, Apr 12, 2016 at 9:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Hi Jun,
>
> I understand the point about the SASL tokens being similar to the SSL
> handshake in a way. However, is there any SASL library that handles the
> network communication for these tokens? I couldn't find any and without
> that, there isn't much benefit in deviating from Kafka's protocol (we
> basically save the space taken by the request header). It's worth
> mentioning that we are already adding the message size before the opaque
> bytes provided by the library, so one could say we are already extending
> the protocol.
>
> If we leave versioning aside, adding the standard Kafka request header to
> those messages may also help from a debugging perspective as would then
> include client id and correlation id along with the message.
>
> Ismael
>
> On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
>
> > Magnus,
> >
> > That sounds reasonable. To reduce the changes on the server side, I'd
> > suggest the following minor tweaks on the proposal.
> >
> > 1. Continue supporting the separate SASL and SASL_SSL port.
> >
> > On SASL port, we support the new sequence
> > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> > regular
> > requests
> >
> > On SASL_SSL port, we support the new sequence
> > SSL handshake bytes, ApiVersionRequest (optional),
> > SaslHandshakeRequest,
> > SASL tokens, regular requests
> >
> > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your argument
> > about SSL handshake, those SASL tokens are generated by SASL library and
> > Kafka doesn't really control its versioning. Kafka only controls the
> > selection of SASL mechanism, which will be versioned in
> > SaslHandshakeRequest.
> >
> > Does that work for you?
> >
> > Thanks,
> >
> > Jun
> >
> >
> > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> > wrote:
> >
> > > Hey Jun, see inline
> > >
> > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > >
> > > > Hi, Magnus,
> > > >
> > > > Let me understand your proposal in more details just from the
> client's
> > > > perspective. My understanding of your proposal is the following.
> > > >
> > > > On plaintext port, the client will send the following bytes in order.
> > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> > > > enabled), regular requests
> > > >
> > > > On SSL port, the client will send the following bytes in order.
> > > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest,
> SASL
> > > > tokens (if SASL is enabled), regular requests
> > > >
> > >
> > >
> > > Yup!
> > > "SASL tokens" is a series of proper Kafka protocol
> SaslHandshakeRequests
> > > until
> > > the handshake is done.
> > >
> > >
> > >
> > > >
> > > > Is that right? Since we can use either SSL or SASL for
> authentication,
> > > it's
> > > > weird that in one case, we require ApiVersionRequest to happen before
> > > > authentication and in another case we require the reverse.
> > > >
> > >
> > > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > > libraries it
> > > doesnt make sense to reimplement that on top of Kafka, so it isn't
> really
> > > comparable.
> > > But for SASL there is no standardised handshake protocol so we must
> > either
> > > conceive one from scratch, or use the protocol that we already have
> > > (Kafka).
> > > For the initial SASL implementation in 0.9 the first option was chosen
> > and
> > > while
> > > it required a new protocol implementation in supporting clients and the
> > > broker
> > > it served its purpose. But not for long, it already needs to evolve,
> > > and this gives us a golden[1] opportunity to make the implementation
> > > reusable, evolvable, less complex
> > > and in line with all our other protocol work, by using the protocol
> > stack
> > > of Kafka which the
> > > broker and all clients already have in place.
> > >
> > > Not taking this chance and instead diverging the custom SASL handshake
> > > protocol
> > > even further from Kafka seems to me a weird choice.
> > >
> > > The current KIP-43 proposal does not have a clear compatibility story;
> it
> > > doesnt seem to be possible
> > > to upgrade clients before brokers, while this might be okay for the
> Java
> > > client, the KIP-35 discussion
> > > has hopefully proven that this assumption can't be made for the entire
> > > eco-system.
> > >
> > > Let me be clear that there isn't anything technically wrong with the
> > KIP-43
> > > proposal (well,
> > > except for the hack to check byte[0] for 0x60 perhaps), but I'm worried
> > the
> > > proposal will eventually lead to
> > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
> handshake,
> > > and this is just redundant,
> > > there is no technical reason for doing so and it'll just make protocol
> > > semantics and implementations more complex.
> > >
> > >
> > > Regards,
> > > Magnus
> > >
> > > [1]: Timing is good for this change since only two clients, Java and C,
> > > currently supports
> > > the existing SASL handshake so far.
> > >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
> magnus@edenhill.se>
> > > > wrote:
> > > >
> > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > > >
> > > > > > Thinking about ApiVersionRequest a bit more. There are quite a
> few
> > > > things
> > > > > > special about it. In the ideal case, (1) its version should never
> > > > change;
> > > > > >
> > > > >
> > > > > The only thing we know of the future is that we dont know anything,
> > we
> > > > > can't
> > > > > think of every possible future use case, that's why need to be able
> > to
> > > > > evolve interfaces
> > > > > as requirements and use-cases change. This is the gist of KIP-35,
> and
> > > > > hampering
> > > > > KIP-35 itself, by not letting it also evolve, does not seem right
> to
> > me
> > > > at
> > > > > all.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > (2) it needs to be done before authentication (either SSL/SASL);
> > (3)
> > > it
> > > > > is
> > > > > > required to be issued at the beginning of each connection but
> never
> > > > needs
> > > > > > to be issued again on the same connection. So, instead of
> modeling
> > it
> > > > as
> > > > > a
> > > > > > regular request, it seems a cleaner approach is to just bake that
> > > into
> > > > > the
> > > > > > initial connection handshake even before the authentication
> layer.
> > So
> > > > the
> > > > > > sequencing in a connection will be api discovery, authentication,
> > > > > followed
> > > > > > by regular requests. I am not sure we can still add this in a
> > > backward
> > > > > > compatible way now (e.g., not sure what the initial bytes from an
> > SSL
> > > > > > connection will look like). Even if we can do this in a backward
> > > > > compatible
> > > > > > way, it's probably non-trivial amount of work though.
> > > > > >
> > > > >
> > > > > I have the luxory of not knowing the broker internals, so I can
> only
> > > > > discuss
> > > > > this on a conceptual design level.
> > > > >
> > > > > In its simplest form each API request type has a NeedsAuth flag and
> > the
> > > > > broker protocol request layer simply checks if the current session
> is
> > > > > Authenticated
> > > > > before processing the request: If not the session is closed and an
> > > error
> > > > is
> > > > > logged.
> > > > > The only two API requests that dont have the NeedsAuth flag would
> be
> > > > > SaslHandshakeRequest
> > > > > and ApiVersionRequest, the latter could also use filtering to only
> > > return
> > > > > the same two
> > > > > requests in ApiVersionResponse before the client is authenticated
> (as
> > > not
> > > > > to "leak" information).
> > > > > If authentication is not configured on the broker all sessions are
> > > deemed
> > > > > authenticated by default.
> > > > >
> > > > >
> > > > > Re backwards compatibility:
> > > > > My suggestion is to keep the current special SASL handshake
> protocol
> > on
> > > > the
> > > > > SASL_PLAIN/SASL_SSL
> > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API
> on
> > > the
> > > > > PLAIN/SSL endpoints.
> > > > > This way the broker is backwards compatible with older clients that
> > > only
> > > > > supports the special SASL protocol,
> > > > > and newer cliets are also backwards compatible with older brokers
> > that
> > > > only
> > > > > supports the special SASL protocol.
> > > > > Newer clients connecting to new brokers will be configured to use
> > > > non-SASL
> > > > > ports and use the
> > > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > > >
> > > > > Using the existing standard Kafka protocol and the new future-proof
> > > > > functionality of ApiVersionRequest
> > > > > allows the in-band authentication mechanisms and semantics to
> > naturally
> > > > > evolve over time
> > > > > without breaking existing clients.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > We started KIP-35 with supporting a client to know if a version
> is
> > > > > > supported by the broker. It's now evolved into supporting a
> client
> > to
> > > > > > implement multiple versions of the protocol and dynamically pick
> a
> > > > > version
> > > > > > supported by the broker. The former is likely solvable without
> > > > > > ApiVersionRequest. How important is the latter? What if the C
> > client
> > > > just
> > > > > > follows the java client model by implementing one version of
> > protocol
> > > > > per C
> > > > > > client release (which seems easier to implement)?
> > > > > >
> > > > >
> > > > > We've discussed this at length and it is not an option for
> > librdkafka,
> > > > nor
> > > > > kafka-python, and
> > > > > probably other clients as well, due to usability/UX and maintenance
> > > > > reasons.
> > > > > (There's even discussion of making the Java client more version
> > > > agnostic!)
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > >
> > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
> wrote:
> > > > > >
> > > > > > > Magnus,
> > > > > > >
> > > > > > > A while back, we had another proposal for the broker to just
> send
> > > the
> > > > > > > correlation id and an empty payload if it receives an
> unsupported
> > > > > version
> > > > > > > of the request. I didn't see that in the rejected section. It
> > seems
> > > > > > simpler
> > > > > > > than the current proposal where the client has to issue an
> > > > > > > ApiVersionRequest on every connection. Could you document the
> > > reason
> > > > > why
> > > > > > we
> > > > > > > rejected it?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> > asingh@cloudera.com>
> > > > > > wrote:
> > > > > > >
> > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > > >>
> > > > > > >> > Two more things:
> > > > > > >> >
> > > > > > >> > 3. We talk about backporting of new request versions to
> stable
> > > > > > branches
> > > > > > >> in
> > > > > > >> > the KIP. In practice, we can't do that until the Java client
> > is
> > > > > > changed
> > > > > > >> so
> > > > > > >> > that it doesn't blindly use the latest protocol version.
> > > > Otherwise,
> > > > > if
> > > > > > >> new
> > > > > > >> > request versions were added to 0.9.0.2, the client would
> break
> > > > when
> > > > > > >> talking
> > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a
> > bit
> > > > > more
> > > > > > >> > gracefully, but that's still not good enough for a stable
> > > branch).
> > > > > It
> > > > > > >> may
> > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > > orthogonal
> > > > > and
> > > > > > >> > doesn't prevent the KIP from being adopted, but good to
> avoid
> > > > > > >> confusion).
> > > > > > >> >
> > > > > > >> Good point. Adding this note and also adding a note that Kafka
> > has
> > > > not
> > > > > > >> backported an api version so far.
> > > > > > >>
> > > > > > >> >
> > > > > > >> > 4. The paragraph below is a bit confusing. It starts talking
> > > about
> > > > > > 0.9.0
> > > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > > >> >
> > > > > > >> Yes.
> > > > > > >>
> > > > > > >> >
> > > > > > >> > "Deprecation of a protocol version will be done by marking a
> > > > > protocol
> > > > > > >> > version as deprecated in protocol documentation.
> Documentation
> > > > shall
> > > > > > >> also
> > > > > > >> > be used to indicate a protocol version that must not be
> used,
> > or
> > > > for
> > > > > > any
> > > > > > >> > such information.For instance, say 0.9.0 had protocol
> versions
> > > [0]
> > > > > for
> > > > > > >> api
> > > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > > running
> > > > > off
> > > > > > >> > trunk started using version 1 of the api and found out a
> major
> > > > bug.
> > > > > To
> > > > > > >> > rectify that version 2 of the api is added to trunk. For
> some
> > > > > reason,
> > > > > > >> it is
> > > > > > >> > now deemed important to have version 2 of the api in 0.9.1
> as
> > > > well.
> > > > > To
> > > > > > >> do
> > > > > > >> > so, version 1 and version 2 both of the api will be
> backported
> > > to
> > > > > the
> > > > > > >> 0.9.1
> > > > > > >> > branch. 0.9.1 broker will return 0 as min supported version
> > for
> > > > the
> > > > > > api
> > > > > > >> and
> > > > > > >> > 2 for the max supported version for the api. However, the
> > > version
> > > > 1
> > > > > > >> should
> > > > > > >> > be clearly marked as deprecated on its documentation. It
> will
> > be
> > > > > > >> client's
> > > > > > >> > responsibility to make sure they are not using any such
> > > deprecated
> > > > > > >> version
> > > > > > >> > to the best knowledge of the client at the time of
> development
> > > (or
> > > > > > >> > alternatively by configuration)."
> > > > > > >> >
> > > > > > >> > Ismael
> > > > > > >> >
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > >> >
> > > > > > >> > > A couple of questions:
> > > > > > >> > >
> > > > > > >> > > 1. The KIP says "Specific version may be deprecated
> through
> > > > > protocol
> > > > > > >> > > documentation but must still be supported (although it is
> > fair
> > > > to
> > > > > > >> return
> > > > > > >> > an
> > > > > > >> > > error code if the specific API supports it).". It may be
> > worth
> > > > > > >> expanding
> > > > > > >> > > this a little more. For example, what does it mean to
> > support
> > > > the
> > > > > > >> API? I
> > > > > > >> > > guess this means that the broker must not disconnect the
> > > client
> > > > > and
> > > > > > >> the
> > > > > > >> > > broker must return a valid protocol response. Given that
> it
> > > says
> > > > > > that
> > > > > > >> it
> > > > > > >> > is
> > > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > > return
> > > > an
> > > > > > >> error
> > > > > > >> > > code if the specific API supports it, it sounds like we
> are
> > > > saying
> > > > > > >> that
> > > > > > >> > we
> > > > > > >> > > don't have to maintain the semantic behaviour (i.e. we
> could
> > > > > > _always_
> > > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > > >> > >
> > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > > >> ApiVersionRequest?
> > > > > > >> > >
> > > > > > >> > > Ismael
> > > > > > >> > >
> > > > > > >> >
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > > >> --
> > > > > > >>
> > > > > > >> Regards,
> > > > > > >> Ashish
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
Regards,
Rajini
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ashish Singh <as...@cloudera.com>.
Done. Thanks!
On Thu, Apr 21, 2016 at 4:32 PM, Gwen Shapira <gw...@confluent.io> wrote:
> Lets start a vote immediately? We are short of time toward the release.
>
> On Thu, Apr 21, 2016 at 2:57 PM, Ashish Singh <as...@cloudera.com> wrote:
> > Hey Guys,
> >
> > KIP-35
> > <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version
> >
> > has been updated based on latest discussions and following PRs have also
> > been updated.
> > 1. KAFKA-3307: Add ApiVersion request/response and server side handling.
> > <https://github.com/apache/kafka/pull/986>
> > 2. KAFKA-3600: Enhance java clients to use ApiVersion Req/Resp to check
> if
> > the broker they are talking to supports required api versions.
> > <https://github.com/apache/kafka/pull/1251>
> >
> > If there are no major objections or changes suggested, we can start a
> vote
> > thread in a couple of days.
> >
> > On Tue, Apr 12, 2016 at 8:04 AM, Jun Rao <ju...@confluent.io> wrote:
> >
> >> Hi, Ismael,
> >>
> >> The SASL engine that we used is the SASL library, right? How did the C
> >> client generate those SASL tokens? Once a SASL mechanism is chosen, the
> >> subsequent tokens are determined, right? So, my feeling is that those
> >> tokens are part of SaslHandshakeRequest and are just extended across
> >> multiple network packets. So modeling those as independent requests
> feels
> >> weird. When documentation them, we really need to document those as a
> >> sequence, not individual isolated requests that can be issued
> >> in arbitrary order. The version id will only add confusion since we
> can't
> >> version the tokens independently. We could explicitly add the client id
> and
> >> correlation id in the header, but I am not sure if it's worth the
> >> additional complexity.
> >>
> >> Thanks,
> >>
> >> Jun
> >>
> >> On Tue, Apr 12, 2016 at 1:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >>
> >> > Hi Jun,
> >> >
> >> > I understand the point about the SASL tokens being similar to the SSL
> >> > handshake in a way. However, is there any SASL library that handles
> the
> >> > network communication for these tokens? I couldn't find any and
> without
> >> > that, there isn't much benefit in deviating from Kafka's protocol (we
> >> > basically save the space taken by the request header). It's worth
> >> > mentioning that we are already adding the message size before the
> opaque
> >> > bytes provided by the library, so one could say we are already
> extending
> >> > the protocol.
> >> >
> >> > If we leave versioning aside, adding the standard Kafka request
> header to
> >> > those messages may also help from a debugging perspective as would
> then
> >> > include client id and correlation id along with the message.
> >> >
> >> > Ismael
> >> >
> >> > On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
> >> >
> >> > > Magnus,
> >> > >
> >> > > That sounds reasonable. To reduce the changes on the server side,
> I'd
> >> > > suggest the following minor tweaks on the proposal.
> >> > >
> >> > > 1. Continue supporting the separate SASL and SASL_SSL port.
> >> > >
> >> > > On SASL port, we support the new sequence
> >> > > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> >> > > regular
> >> > > requests
> >> > >
> >> > > On SASL_SSL port, we support the new sequence
> >> > > SSL handshake bytes, ApiVersionRequest (optional),
> >> > > SaslHandshakeRequest,
> >> > > SASL tokens, regular requests
> >> > >
> >> > > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your
> >> argument
> >> > > about SSL handshake, those SASL tokens are generated by SASL library
> >> and
> >> > > Kafka doesn't really control its versioning. Kafka only controls the
> >> > > selection of SASL mechanism, which will be versioned in
> >> > > SaslHandshakeRequest.
> >> > >
> >> > > Does that work for you?
> >> > >
> >> > > Thanks,
> >> > >
> >> > > Jun
> >> > >
> >> > >
> >> > > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <
> magnus@edenhill.se>
> >> > > wrote:
> >> > >
> >> > > > Hey Jun, see inline
> >> > > >
> >> > > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> >> > > >
> >> > > > > Hi, Magnus,
> >> > > > >
> >> > > > > Let me understand your proposal in more details just from the
> >> > client's
> >> > > > > perspective. My understanding of your proposal is the following.
> >> > > > >
> >> > > > > On plaintext port, the client will send the following bytes in
> >> order.
> >> > > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if
> SASL
> >> is
> >> > > > > enabled), regular requests
> >> > > > >
> >> > > > > On SSL port, the client will send the following bytes in order.
> >> > > > > SSL handshake bytes, ApiVersionRequest,
> SaslHandshakeRequest,
> >> > SASL
> >> > > > > tokens (if SASL is enabled), regular requests
> >> > > > >
> >> > > >
> >> > > >
> >> > > > Yup!
> >> > > > "SASL tokens" is a series of proper Kafka protocol
> >> > SaslHandshakeRequests
> >> > > > until
> >> > > > the handshake is done.
> >> > > >
> >> > > >
> >> > > >
> >> > > > >
> >> > > > > Is that right? Since we can use either SSL or SASL for
> >> > authentication,
> >> > > > it's
> >> > > > > weird that in one case, we require ApiVersionRequest to happen
> >> before
> >> > > > > authentication and in another case we require the reverse.
> >> > > > >
> >> > > >
> >> > > > Since the SSL/TLS is standardised and taken care of for us by the
> SSL
> >> > > > libraries it
> >> > > > doesnt make sense to reimplement that on top of Kafka, so it isn't
> >> > really
> >> > > > comparable.
> >> > > > But for SASL there is no standardised handshake protocol so we
> must
> >> > > either
> >> > > > conceive one from scratch, or use the protocol that we already
> have
> >> > > > (Kafka).
> >> > > > For the initial SASL implementation in 0.9 the first option was
> >> chosen
> >> > > and
> >> > > > while
> >> > > > it required a new protocol implementation in supporting clients
> and
> >> the
> >> > > > broker
> >> > > > it served its purpose. But not for long, it already needs to
> evolve,
> >> > > > and this gives us a golden[1] opportunity to make the
> implementation
> >> > > > reusable, evolvable, less complex
> >> > > > and in line with all our other protocol work, by using the
> protocol
> >> > > stack
> >> > > > of Kafka which the
> >> > > > broker and all clients already have in place.
> >> > > >
> >> > > > Not taking this chance and instead diverging the custom SASL
> >> handshake
> >> > > > protocol
> >> > > > even further from Kafka seems to me a weird choice.
> >> > > >
> >> > > > The current KIP-43 proposal does not have a clear compatibility
> >> story;
> >> > it
> >> > > > doesnt seem to be possible
> >> > > > to upgrade clients before brokers, while this might be okay for
> the
> >> > Java
> >> > > > client, the KIP-35 discussion
> >> > > > has hopefully proven that this assumption can't be made for the
> >> entire
> >> > > > eco-system.
> >> > > >
> >> > > > Let me be clear that there isn't anything technically wrong with
> the
> >> > > KIP-43
> >> > > > proposal (well,
> >> > > > except for the hack to check byte[0] for 0x60 perhaps), but I'm
> >> worried
> >> > > the
> >> > > > proposal will eventually lead to
> >> > > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
> >> > handshake,
> >> > > > and this is just redundant,
> >> > > > there is no technical reason for doing so and it'll just make
> >> protocol
> >> > > > semantics and implementations more complex.
> >> > > >
> >> > > >
> >> > > > Regards,
> >> > > > Magnus
> >> > > >
> >> > > > [1]: Timing is good for this change since only two clients, Java
> and
> >> C,
> >> > > > currently supports
> >> > > > the existing SASL handshake so far.
> >> > > >
> >> > > >
> >> > > > >
> >> > > > > Thanks,
> >> > > > >
> >> > > > > Jun
> >> > > > >
> >> > > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
> >> > magnus@edenhill.se>
> >> > > > > wrote:
> >> > > > >
> >> > > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> >> > > > > >
> >> > > > > > > Thinking about ApiVersionRequest a bit more. There are
> quite a
> >> > few
> >> > > > > things
> >> > > > > > > special about it. In the ideal case, (1) its version should
> >> never
> >> > > > > change;
> >> > > > > > >
> >> > > > > >
> >> > > > > > The only thing we know of the future is that we dont know
> >> anything,
> >> > > we
> >> > > > > > can't
> >> > > > > > think of every possible future use case, that's why need to be
> >> able
> >> > > to
> >> > > > > > evolve interfaces
> >> > > > > > as requirements and use-cases change. This is the gist of
> KIP-35,
> >> > and
> >> > > > > > hampering
> >> > > > > > KIP-35 itself, by not letting it also evolve, does not seem
> right
> >> > to
> >> > > me
> >> > > > > at
> >> > > > > > all.
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > > (2) it needs to be done before authentication (either
> >> SSL/SASL);
> >> > > (3)
> >> > > > it
> >> > > > > > is
> >> > > > > > > required to be issued at the beginning of each connection
> but
> >> > never
> >> > > > > needs
> >> > > > > > > to be issued again on the same connection. So, instead of
> >> > modeling
> >> > > it
> >> > > > > as
> >> > > > > > a
> >> > > > > > > regular request, it seems a cleaner approach is to just bake
> >> that
> >> > > > into
> >> > > > > > the
> >> > > > > > > initial connection handshake even before the authentication
> >> > layer.
> >> > > So
> >> > > > > the
> >> > > > > > > sequencing in a connection will be api discovery,
> >> authentication,
> >> > > > > > followed
> >> > > > > > > by regular requests. I am not sure we can still add this in
> a
> >> > > > backward
> >> > > > > > > compatible way now (e.g., not sure what the initial bytes
> from
> >> an
> >> > > SSL
> >> > > > > > > connection will look like). Even if we can do this in a
> >> backward
> >> > > > > > compatible
> >> > > > > > > way, it's probably non-trivial amount of work though.
> >> > > > > > >
> >> > > > > >
> >> > > > > > I have the luxory of not knowing the broker internals, so I
> can
> >> > only
> >> > > > > > discuss
> >> > > > > > this on a conceptual design level.
> >> > > > > >
> >> > > > > > In its simplest form each API request type has a NeedsAuth
> flag
> >> and
> >> > > the
> >> > > > > > broker protocol request layer simply checks if the current
> >> session
> >> > is
> >> > > > > > Authenticated
> >> > > > > > before processing the request: If not the session is closed
> and
> >> an
> >> > > > error
> >> > > > > is
> >> > > > > > logged.
> >> > > > > > The only two API requests that dont have the NeedsAuth flag
> would
> >> > be
> >> > > > > > SaslHandshakeRequest
> >> > > > > > and ApiVersionRequest, the latter could also use filtering to
> >> only
> >> > > > return
> >> > > > > > the same two
> >> > > > > > requests in ApiVersionResponse before the client is
> authenticated
> >> > (as
> >> > > > not
> >> > > > > > to "leak" information).
> >> > > > > > If authentication is not configured on the broker all sessions
> >> are
> >> > > > deemed
> >> > > > > > authenticated by default.
> >> > > > > >
> >> > > > > >
> >> > > > > > Re backwards compatibility:
> >> > > > > > My suggestion is to keep the current special SASL handshake
> >> > protocol
> >> > > on
> >> > > > > the
> >> > > > > > SASL_PLAIN/SASL_SSL
> >> > > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest
> API
> >> > on
> >> > > > the
> >> > > > > > PLAIN/SSL endpoints.
> >> > > > > > This way the broker is backwards compatible with older clients
> >> that
> >> > > > only
> >> > > > > > supports the special SASL protocol,
> >> > > > > > and newer cliets are also backwards compatible with older
> brokers
> >> > > that
> >> > > > > only
> >> > > > > > supports the special SASL protocol.
> >> > > > > > Newer clients connecting to new brokers will be configured to
> use
> >> > > > > non-SASL
> >> > > > > > ports and use the
> >> > > > > > in-band Kafka SaslHandshakeRequest to authenticate.
> >> > > > > >
> >> > > > > > Using the existing standard Kafka protocol and the new
> >> future-proof
> >> > > > > > functionality of ApiVersionRequest
> >> > > > > > allows the in-band authentication mechanisms and semantics to
> >> > > naturally
> >> > > > > > evolve over time
> >> > > > > > without breaking existing clients.
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > >
> >> > > > > > > We started KIP-35 with supporting a client to know if a
> version
> >> > is
> >> > > > > > > supported by the broker. It's now evolved into supporting a
> >> > client
> >> > > to
> >> > > > > > > implement multiple versions of the protocol and dynamically
> >> pick
> >> > a
> >> > > > > > version
> >> > > > > > > supported by the broker. The former is likely solvable
> without
> >> > > > > > > ApiVersionRequest. How important is the latter? What if the
> C
> >> > > client
> >> > > > > just
> >> > > > > > > follows the java client model by implementing one version of
> >> > > protocol
> >> > > > > > per C
> >> > > > > > > client release (which seems easier to implement)?
> >> > > > > > >
> >> > > > > >
> >> > > > > > We've discussed this at length and it is not an option for
> >> > > librdkafka,
> >> > > > > nor
> >> > > > > > kafka-python, and
> >> > > > > > probably other clients as well, due to usability/UX and
> >> maintenance
> >> > > > > > reasons.
> >> > > > > > (There's even discussion of making the Java client more
> version
> >> > > > > agnostic!)
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > >
> >> > > > > > > Thanks,
> >> > > > > > >
> >> > > > > > > Jun
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
> >> > wrote:
> >> > > > > > >
> >> > > > > > > > Magnus,
> >> > > > > > > >
> >> > > > > > > > A while back, we had another proposal for the broker to
> just
> >> > send
> >> > > > the
> >> > > > > > > > correlation id and an empty payload if it receives an
> >> > unsupported
> >> > > > > > version
> >> > > > > > > > of the request. I didn't see that in the rejected
> section. It
> >> > > seems
> >> > > > > > > simpler
> >> > > > > > > > than the current proposal where the client has to issue an
> >> > > > > > > > ApiVersionRequest on every connection. Could you document
> the
> >> > > > reason
> >> > > > > > why
> >> > > > > > > we
> >> > > > > > > > rejected it?
> >> > > > > > > >
> >> > > > > > > > Thanks,
> >> > > > > > > >
> >> > > > > > > > Jun
> >> > > > > > > >
> >> > > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> >> > > asingh@cloudera.com>
> >> > > > > > > wrote:
> >> > > > > > > >
> >> > > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
> >> > ismael@juma.me.uk>
> >> > > > > > wrote:
> >> > > > > > > >>
> >> > > > > > > >> > Two more things:
> >> > > > > > > >> >
> >> > > > > > > >> > 3. We talk about backporting of new request versions to
> >> > stable
> >> > > > > > > branches
> >> > > > > > > >> in
> >> > > > > > > >> > the KIP. In practice, we can't do that until the Java
> >> client
> >> > > is
> >> > > > > > > changed
> >> > > > > > > >> so
> >> > > > > > > >> > that it doesn't blindly use the latest protocol
> version.
> >> > > > > Otherwise,
> >> > > > > > if
> >> > > > > > > >> new
> >> > > > > > > >> > request versions were added to 0.9.0.2, the client
> would
> >> > break
> >> > > > > when
> >> > > > > > > >> talking
> >> > > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would
> >> fail a
> >> > > bit
> >> > > > > > more
> >> > > > > > > >> > gracefully, but that's still not good enough for a
> stable
> >> > > > branch).
> >> > > > > > It
> >> > > > > > > >> may
> >> > > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> >> > > > orthogonal
> >> > > > > > and
> >> > > > > > > >> > doesn't prevent the KIP from being adopted, but good to
> >> > avoid
> >> > > > > > > >> confusion).
> >> > > > > > > >> >
> >> > > > > > > >> Good point. Adding this note and also adding a note that
> >> Kafka
> >> > > has
> >> > > > > not
> >> > > > > > > >> backported an api version so far.
> >> > > > > > > >>
> >> > > > > > > >> >
> >> > > > > > > >> > 4. The paragraph below is a bit confusing. It starts
> >> talking
> >> > > > about
> >> > > > > > > 0.9.0
> >> > > > > > > >> > and trunk and then switches to 0.9.1. Is that
> intentional?
> >> > > > > > > >> >
> >> > > > > > > >> Yes.
> >> > > > > > > >>
> >> > > > > > > >> >
> >> > > > > > > >> > "Deprecation of a protocol version will be done by
> >> marking a
> >> > > > > > protocol
> >> > > > > > > >> > version as deprecated in protocol documentation.
> >> > Documentation
> >> > > > > shall
> >> > > > > > > >> also
> >> > > > > > > >> > be used to indicate a protocol version that must not be
> >> > used,
> >> > > or
> >> > > > > for
> >> > > > > > > any
> >> > > > > > > >> > such information.For instance, say 0.9.0 had protocol
> >> > versions
> >> > > > [0]
> >> > > > > > for
> >> > > > > > > >> api
> >> > > > > > > >> > key 1. On trunk, version 1 of the api key was added.
> Users
> >> > > > running
> >> > > > > > off
> >> > > > > > > >> > trunk started using version 1 of the api and found out
> a
> >> > major
> >> > > > > bug.
> >> > > > > > To
> >> > > > > > > >> > rectify that version 2 of the api is added to trunk.
> For
> >> > some
> >> > > > > > reason,
> >> > > > > > > >> it is
> >> > > > > > > >> > now deemed important to have version 2 of the api in
> 0.9.1
> >> > as
> >> > > > > well.
> >> > > > > > To
> >> > > > > > > >> do
> >> > > > > > > >> > so, version 1 and version 2 both of the api will be
> >> > backported
> >> > > > to
> >> > > > > > the
> >> > > > > > > >> 0.9.1
> >> > > > > > > >> > branch. 0.9.1 broker will return 0 as min supported
> >> version
> >> > > for
> >> > > > > the
> >> > > > > > > api
> >> > > > > > > >> and
> >> > > > > > > >> > 2 for the max supported version for the api. However,
> the
> >> > > > version
> >> > > > > 1
> >> > > > > > > >> should
> >> > > > > > > >> > be clearly marked as deprecated on its documentation.
> It
> >> > will
> >> > > be
> >> > > > > > > >> client's
> >> > > > > > > >> > responsibility to make sure they are not using any such
> >> > > > deprecated
> >> > > > > > > >> version
> >> > > > > > > >> > to the best knowledge of the client at the time of
> >> > development
> >> > > > (or
> >> > > > > > > >> > alternatively by configuration)."
> >> > > > > > > >> >
> >> > > > > > > >> > Ismael
> >> > > > > > > >> >
> >> > > > > > > >> >
> >> > > > > > > >> >
> >> > > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> >> > > ismael@juma.me.uk>
> >> > > > > > > wrote:
> >> > > > > > > >> >
> >> > > > > > > >> > > A couple of questions:
> >> > > > > > > >> > >
> >> > > > > > > >> > > 1. The KIP says "Specific version may be deprecated
> >> > through
> >> > > > > > protocol
> >> > > > > > > >> > > documentation but must still be supported (although
> it
> >> is
> >> > > fair
> >> > > > > to
> >> > > > > > > >> return
> >> > > > > > > >> > an
> >> > > > > > > >> > > error code if the specific API supports it).". It
> may be
> >> > > worth
> >> > > > > > > >> expanding
> >> > > > > > > >> > > this a little more. For example, what does it mean to
> >> > > support
> >> > > > > the
> >> > > > > > > >> API? I
> >> > > > > > > >> > > guess this means that the broker must not disconnect
> the
> >> > > > client
> >> > > > > > and
> >> > > > > > > >> the
> >> > > > > > > >> > > broker must return a valid protocol response. Given
> that
> >> > it
> >> > > > says
> >> > > > > > > that
> >> > > > > > > >> it
> >> > > > > > > >> > is
> >> > > > > > > >> > > "fair" (I would probably replace "fair" with
> "valid") to
> >> > > > return
> >> > > > > an
> >> > > > > > > >> error
> >> > > > > > > >> > > code if the specific API supports it, it sounds like
> we
> >> > are
> >> > > > > saying
> >> > > > > > > >> that
> >> > > > > > > >> > we
> >> > > > > > > >> > > don't have to maintain the semantic behaviour (i.e.
> we
> >> > could
> >> > > > > > > _always_
> >> > > > > > > >> > > return an error for a deprecated API?). Is this true?
> >> > > > > > > >> > >
> >> > > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why
> not
> >> > > > > > > >> ApiVersionRequest?
> >> > > > > > > >> > >
> >> > > > > > > >> > > Ismael
> >> > > > > > > >> > >
> >> > > > > > > >> >
> >> > > > > > > >>
> >> > > > > > > >>
> >> > > > > > > >>
> >> > > > > > > >> --
> >> > > > > > > >>
> >> > > > > > > >> Regards,
> >> > > > > > > >> Ashish
> >> > > > > > > >>
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
>
--
Regards,
Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Gwen Shapira <gw...@confluent.io>.
Lets start a vote immediately? We are short of time toward the release.
On Thu, Apr 21, 2016 at 2:57 PM, Ashish Singh <as...@cloudera.com> wrote:
> Hey Guys,
>
> KIP-35
> <https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version>
> has been updated based on latest discussions and following PRs have also
> been updated.
> 1. KAFKA-3307: Add ApiVersion request/response and server side handling.
> <https://github.com/apache/kafka/pull/986>
> 2. KAFKA-3600: Enhance java clients to use ApiVersion Req/Resp to check if
> the broker they are talking to supports required api versions.
> <https://github.com/apache/kafka/pull/1251>
>
> If there are no major objections or changes suggested, we can start a vote
> thread in a couple of days.
>
> On Tue, Apr 12, 2016 at 8:04 AM, Jun Rao <ju...@confluent.io> wrote:
>
>> Hi, Ismael,
>>
>> The SASL engine that we used is the SASL library, right? How did the C
>> client generate those SASL tokens? Once a SASL mechanism is chosen, the
>> subsequent tokens are determined, right? So, my feeling is that those
>> tokens are part of SaslHandshakeRequest and are just extended across
>> multiple network packets. So modeling those as independent requests feels
>> weird. When documentation them, we really need to document those as a
>> sequence, not individual isolated requests that can be issued
>> in arbitrary order. The version id will only add confusion since we can't
>> version the tokens independently. We could explicitly add the client id and
>> correlation id in the header, but I am not sure if it's worth the
>> additional complexity.
>>
>> Thanks,
>>
>> Jun
>>
>> On Tue, Apr 12, 2016 at 1:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
>>
>> > Hi Jun,
>> >
>> > I understand the point about the SASL tokens being similar to the SSL
>> > handshake in a way. However, is there any SASL library that handles the
>> > network communication for these tokens? I couldn't find any and without
>> > that, there isn't much benefit in deviating from Kafka's protocol (we
>> > basically save the space taken by the request header). It's worth
>> > mentioning that we are already adding the message size before the opaque
>> > bytes provided by the library, so one could say we are already extending
>> > the protocol.
>> >
>> > If we leave versioning aside, adding the standard Kafka request header to
>> > those messages may also help from a debugging perspective as would then
>> > include client id and correlation id along with the message.
>> >
>> > Ismael
>> >
>> > On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
>> >
>> > > Magnus,
>> > >
>> > > That sounds reasonable. To reduce the changes on the server side, I'd
>> > > suggest the following minor tweaks on the proposal.
>> > >
>> > > 1. Continue supporting the separate SASL and SASL_SSL port.
>> > >
>> > > On SASL port, we support the new sequence
>> > > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
>> > > regular
>> > > requests
>> > >
>> > > On SASL_SSL port, we support the new sequence
>> > > SSL handshake bytes, ApiVersionRequest (optional),
>> > > SaslHandshakeRequest,
>> > > SASL tokens, regular requests
>> > >
>> > > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your
>> argument
>> > > about SSL handshake, those SASL tokens are generated by SASL library
>> and
>> > > Kafka doesn't really control its versioning. Kafka only controls the
>> > > selection of SASL mechanism, which will be versioned in
>> > > SaslHandshakeRequest.
>> > >
>> > > Does that work for you?
>> > >
>> > > Thanks,
>> > >
>> > > Jun
>> > >
>> > >
>> > > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
>> > > wrote:
>> > >
>> > > > Hey Jun, see inline
>> > > >
>> > > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
>> > > >
>> > > > > Hi, Magnus,
>> > > > >
>> > > > > Let me understand your proposal in more details just from the
>> > client's
>> > > > > perspective. My understanding of your proposal is the following.
>> > > > >
>> > > > > On plaintext port, the client will send the following bytes in
>> order.
>> > > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL
>> is
>> > > > > enabled), regular requests
>> > > > >
>> > > > > On SSL port, the client will send the following bytes in order.
>> > > > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest,
>> > SASL
>> > > > > tokens (if SASL is enabled), regular requests
>> > > > >
>> > > >
>> > > >
>> > > > Yup!
>> > > > "SASL tokens" is a series of proper Kafka protocol
>> > SaslHandshakeRequests
>> > > > until
>> > > > the handshake is done.
>> > > >
>> > > >
>> > > >
>> > > > >
>> > > > > Is that right? Since we can use either SSL or SASL for
>> > authentication,
>> > > > it's
>> > > > > weird that in one case, we require ApiVersionRequest to happen
>> before
>> > > > > authentication and in another case we require the reverse.
>> > > > >
>> > > >
>> > > > Since the SSL/TLS is standardised and taken care of for us by the SSL
>> > > > libraries it
>> > > > doesnt make sense to reimplement that on top of Kafka, so it isn't
>> > really
>> > > > comparable.
>> > > > But for SASL there is no standardised handshake protocol so we must
>> > > either
>> > > > conceive one from scratch, or use the protocol that we already have
>> > > > (Kafka).
>> > > > For the initial SASL implementation in 0.9 the first option was
>> chosen
>> > > and
>> > > > while
>> > > > it required a new protocol implementation in supporting clients and
>> the
>> > > > broker
>> > > > it served its purpose. But not for long, it already needs to evolve,
>> > > > and this gives us a golden[1] opportunity to make the implementation
>> > > > reusable, evolvable, less complex
>> > > > and in line with all our other protocol work, by using the protocol
>> > > stack
>> > > > of Kafka which the
>> > > > broker and all clients already have in place.
>> > > >
>> > > > Not taking this chance and instead diverging the custom SASL
>> handshake
>> > > > protocol
>> > > > even further from Kafka seems to me a weird choice.
>> > > >
>> > > > The current KIP-43 proposal does not have a clear compatibility
>> story;
>> > it
>> > > > doesnt seem to be possible
>> > > > to upgrade clients before brokers, while this might be okay for the
>> > Java
>> > > > client, the KIP-35 discussion
>> > > > has hopefully proven that this assumption can't be made for the
>> entire
>> > > > eco-system.
>> > > >
>> > > > Let me be clear that there isn't anything technically wrong with the
>> > > KIP-43
>> > > > proposal (well,
>> > > > except for the hack to check byte[0] for 0x60 perhaps), but I'm
>> worried
>> > > the
>> > > > proposal will eventually lead to
>> > > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
>> > handshake,
>> > > > and this is just redundant,
>> > > > there is no technical reason for doing so and it'll just make
>> protocol
>> > > > semantics and implementations more complex.
>> > > >
>> > > >
>> > > > Regards,
>> > > > Magnus
>> > > >
>> > > > [1]: Timing is good for this change since only two clients, Java and
>> C,
>> > > > currently supports
>> > > > the existing SASL handshake so far.
>> > > >
>> > > >
>> > > > >
>> > > > > Thanks,
>> > > > >
>> > > > > Jun
>> > > > >
>> > > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
>> > magnus@edenhill.se>
>> > > > > wrote:
>> > > > >
>> > > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
>> > > > > >
>> > > > > > > Thinking about ApiVersionRequest a bit more. There are quite a
>> > few
>> > > > > things
>> > > > > > > special about it. In the ideal case, (1) its version should
>> never
>> > > > > change;
>> > > > > > >
>> > > > > >
>> > > > > > The only thing we know of the future is that we dont know
>> anything,
>> > > we
>> > > > > > can't
>> > > > > > think of every possible future use case, that's why need to be
>> able
>> > > to
>> > > > > > evolve interfaces
>> > > > > > as requirements and use-cases change. This is the gist of KIP-35,
>> > and
>> > > > > > hampering
>> > > > > > KIP-35 itself, by not letting it also evolve, does not seem right
>> > to
>> > > me
>> > > > > at
>> > > > > > all.
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > > (2) it needs to be done before authentication (either
>> SSL/SASL);
>> > > (3)
>> > > > it
>> > > > > > is
>> > > > > > > required to be issued at the beginning of each connection but
>> > never
>> > > > > needs
>> > > > > > > to be issued again on the same connection. So, instead of
>> > modeling
>> > > it
>> > > > > as
>> > > > > > a
>> > > > > > > regular request, it seems a cleaner approach is to just bake
>> that
>> > > > into
>> > > > > > the
>> > > > > > > initial connection handshake even before the authentication
>> > layer.
>> > > So
>> > > > > the
>> > > > > > > sequencing in a connection will be api discovery,
>> authentication,
>> > > > > > followed
>> > > > > > > by regular requests. I am not sure we can still add this in a
>> > > > backward
>> > > > > > > compatible way now (e.g., not sure what the initial bytes from
>> an
>> > > SSL
>> > > > > > > connection will look like). Even if we can do this in a
>> backward
>> > > > > > compatible
>> > > > > > > way, it's probably non-trivial amount of work though.
>> > > > > > >
>> > > > > >
>> > > > > > I have the luxory of not knowing the broker internals, so I can
>> > only
>> > > > > > discuss
>> > > > > > this on a conceptual design level.
>> > > > > >
>> > > > > > In its simplest form each API request type has a NeedsAuth flag
>> and
>> > > the
>> > > > > > broker protocol request layer simply checks if the current
>> session
>> > is
>> > > > > > Authenticated
>> > > > > > before processing the request: If not the session is closed and
>> an
>> > > > error
>> > > > > is
>> > > > > > logged.
>> > > > > > The only two API requests that dont have the NeedsAuth flag would
>> > be
>> > > > > > SaslHandshakeRequest
>> > > > > > and ApiVersionRequest, the latter could also use filtering to
>> only
>> > > > return
>> > > > > > the same two
>> > > > > > requests in ApiVersionResponse before the client is authenticated
>> > (as
>> > > > not
>> > > > > > to "leak" information).
>> > > > > > If authentication is not configured on the broker all sessions
>> are
>> > > > deemed
>> > > > > > authenticated by default.
>> > > > > >
>> > > > > >
>> > > > > > Re backwards compatibility:
>> > > > > > My suggestion is to keep the current special SASL handshake
>> > protocol
>> > > on
>> > > > > the
>> > > > > > SASL_PLAIN/SASL_SSL
>> > > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API
>> > on
>> > > > the
>> > > > > > PLAIN/SSL endpoints.
>> > > > > > This way the broker is backwards compatible with older clients
>> that
>> > > > only
>> > > > > > supports the special SASL protocol,
>> > > > > > and newer cliets are also backwards compatible with older brokers
>> > > that
>> > > > > only
>> > > > > > supports the special SASL protocol.
>> > > > > > Newer clients connecting to new brokers will be configured to use
>> > > > > non-SASL
>> > > > > > ports and use the
>> > > > > > in-band Kafka SaslHandshakeRequest to authenticate.
>> > > > > >
>> > > > > > Using the existing standard Kafka protocol and the new
>> future-proof
>> > > > > > functionality of ApiVersionRequest
>> > > > > > allows the in-band authentication mechanisms and semantics to
>> > > naturally
>> > > > > > evolve over time
>> > > > > > without breaking existing clients.
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > >
>> > > > > > > We started KIP-35 with supporting a client to know if a version
>> > is
>> > > > > > > supported by the broker. It's now evolved into supporting a
>> > client
>> > > to
>> > > > > > > implement multiple versions of the protocol and dynamically
>> pick
>> > a
>> > > > > > version
>> > > > > > > supported by the broker. The former is likely solvable without
>> > > > > > > ApiVersionRequest. How important is the latter? What if the C
>> > > client
>> > > > > just
>> > > > > > > follows the java client model by implementing one version of
>> > > protocol
>> > > > > > per C
>> > > > > > > client release (which seems easier to implement)?
>> > > > > > >
>> > > > > >
>> > > > > > We've discussed this at length and it is not an option for
>> > > librdkafka,
>> > > > > nor
>> > > > > > kafka-python, and
>> > > > > > probably other clients as well, due to usability/UX and
>> maintenance
>> > > > > > reasons.
>> > > > > > (There's even discussion of making the Java client more version
>> > > > > agnostic!)
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > >
>> > > > > > > Thanks,
>> > > > > > >
>> > > > > > > Jun
>> > > > > > >
>> > > > > > >
>> > > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
>> > wrote:
>> > > > > > >
>> > > > > > > > Magnus,
>> > > > > > > >
>> > > > > > > > A while back, we had another proposal for the broker to just
>> > send
>> > > > the
>> > > > > > > > correlation id and an empty payload if it receives an
>> > unsupported
>> > > > > > version
>> > > > > > > > of the request. I didn't see that in the rejected section. It
>> > > seems
>> > > > > > > simpler
>> > > > > > > > than the current proposal where the client has to issue an
>> > > > > > > > ApiVersionRequest on every connection. Could you document the
>> > > > reason
>> > > > > > why
>> > > > > > > we
>> > > > > > > > rejected it?
>> > > > > > > >
>> > > > > > > > Thanks,
>> > > > > > > >
>> > > > > > > > Jun
>> > > > > > > >
>> > > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
>> > > asingh@cloudera.com>
>> > > > > > > wrote:
>> > > > > > > >
>> > > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
>> > ismael@juma.me.uk>
>> > > > > > wrote:
>> > > > > > > >>
>> > > > > > > >> > Two more things:
>> > > > > > > >> >
>> > > > > > > >> > 3. We talk about backporting of new request versions to
>> > stable
>> > > > > > > branches
>> > > > > > > >> in
>> > > > > > > >> > the KIP. In practice, we can't do that until the Java
>> client
>> > > is
>> > > > > > > changed
>> > > > > > > >> so
>> > > > > > > >> > that it doesn't blindly use the latest protocol version.
>> > > > > Otherwise,
>> > > > > > if
>> > > > > > > >> new
>> > > > > > > >> > request versions were added to 0.9.0.2, the client would
>> > break
>> > > > > when
>> > > > > > > >> talking
>> > > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would
>> fail a
>> > > bit
>> > > > > > more
>> > > > > > > >> > gracefully, but that's still not good enough for a stable
>> > > > branch).
>> > > > > > It
>> > > > > > > >> may
>> > > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
>> > > > orthogonal
>> > > > > > and
>> > > > > > > >> > doesn't prevent the KIP from being adopted, but good to
>> > avoid
>> > > > > > > >> confusion).
>> > > > > > > >> >
>> > > > > > > >> Good point. Adding this note and also adding a note that
>> Kafka
>> > > has
>> > > > > not
>> > > > > > > >> backported an api version so far.
>> > > > > > > >>
>> > > > > > > >> >
>> > > > > > > >> > 4. The paragraph below is a bit confusing. It starts
>> talking
>> > > > about
>> > > > > > > 0.9.0
>> > > > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
>> > > > > > > >> >
>> > > > > > > >> Yes.
>> > > > > > > >>
>> > > > > > > >> >
>> > > > > > > >> > "Deprecation of a protocol version will be done by
>> marking a
>> > > > > > protocol
>> > > > > > > >> > version as deprecated in protocol documentation.
>> > Documentation
>> > > > > shall
>> > > > > > > >> also
>> > > > > > > >> > be used to indicate a protocol version that must not be
>> > used,
>> > > or
>> > > > > for
>> > > > > > > any
>> > > > > > > >> > such information.For instance, say 0.9.0 had protocol
>> > versions
>> > > > [0]
>> > > > > > for
>> > > > > > > >> api
>> > > > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
>> > > > running
>> > > > > > off
>> > > > > > > >> > trunk started using version 1 of the api and found out a
>> > major
>> > > > > bug.
>> > > > > > To
>> > > > > > > >> > rectify that version 2 of the api is added to trunk. For
>> > some
>> > > > > > reason,
>> > > > > > > >> it is
>> > > > > > > >> > now deemed important to have version 2 of the api in 0.9.1
>> > as
>> > > > > well.
>> > > > > > To
>> > > > > > > >> do
>> > > > > > > >> > so, version 1 and version 2 both of the api will be
>> > backported
>> > > > to
>> > > > > > the
>> > > > > > > >> 0.9.1
>> > > > > > > >> > branch. 0.9.1 broker will return 0 as min supported
>> version
>> > > for
>> > > > > the
>> > > > > > > api
>> > > > > > > >> and
>> > > > > > > >> > 2 for the max supported version for the api. However, the
>> > > > version
>> > > > > 1
>> > > > > > > >> should
>> > > > > > > >> > be clearly marked as deprecated on its documentation. It
>> > will
>> > > be
>> > > > > > > >> client's
>> > > > > > > >> > responsibility to make sure they are not using any such
>> > > > deprecated
>> > > > > > > >> version
>> > > > > > > >> > to the best knowledge of the client at the time of
>> > development
>> > > > (or
>> > > > > > > >> > alternatively by configuration)."
>> > > > > > > >> >
>> > > > > > > >> > Ismael
>> > > > > > > >> >
>> > > > > > > >> >
>> > > > > > > >> >
>> > > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
>> > > ismael@juma.me.uk>
>> > > > > > > wrote:
>> > > > > > > >> >
>> > > > > > > >> > > A couple of questions:
>> > > > > > > >> > >
>> > > > > > > >> > > 1. The KIP says "Specific version may be deprecated
>> > through
>> > > > > > protocol
>> > > > > > > >> > > documentation but must still be supported (although it
>> is
>> > > fair
>> > > > > to
>> > > > > > > >> return
>> > > > > > > >> > an
>> > > > > > > >> > > error code if the specific API supports it).". It may be
>> > > worth
>> > > > > > > >> expanding
>> > > > > > > >> > > this a little more. For example, what does it mean to
>> > > support
>> > > > > the
>> > > > > > > >> API? I
>> > > > > > > >> > > guess this means that the broker must not disconnect the
>> > > > client
>> > > > > > and
>> > > > > > > >> the
>> > > > > > > >> > > broker must return a valid protocol response. Given that
>> > it
>> > > > says
>> > > > > > > that
>> > > > > > > >> it
>> > > > > > > >> > is
>> > > > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
>> > > > return
>> > > > > an
>> > > > > > > >> error
>> > > > > > > >> > > code if the specific API supports it, it sounds like we
>> > are
>> > > > > saying
>> > > > > > > >> that
>> > > > > > > >> > we
>> > > > > > > >> > > don't have to maintain the semantic behaviour (i.e. we
>> > could
>> > > > > > > _always_
>> > > > > > > >> > > return an error for a deprecated API?). Is this true?
>> > > > > > > >> > >
>> > > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
>> > > > > > > >> ApiVersionRequest?
>> > > > > > > >> > >
>> > > > > > > >> > > Ismael
>> > > > > > > >> > >
>> > > > > > > >> >
>> > > > > > > >>
>> > > > > > > >>
>> > > > > > > >>
>> > > > > > > >> --
>> > > > > > > >>
>> > > > > > > >> Regards,
>> > > > > > > >> Ashish
>> > > > > > > >>
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
>
> --
>
> Regards,
> Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ashish Singh <as...@cloudera.com>.
Hey Guys,
KIP-35
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version>
has been updated based on latest discussions and following PRs have also
been updated.
1. KAFKA-3307: Add ApiVersion request/response and server side handling.
<https://github.com/apache/kafka/pull/986>
2. KAFKA-3600: Enhance java clients to use ApiVersion Req/Resp to check if
the broker they are talking to supports required api versions.
<https://github.com/apache/kafka/pull/1251>
If there are no major objections or changes suggested, we can start a vote
thread in a couple of days.
On Tue, Apr 12, 2016 at 8:04 AM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Ismael,
>
> The SASL engine that we used is the SASL library, right? How did the C
> client generate those SASL tokens? Once a SASL mechanism is chosen, the
> subsequent tokens are determined, right? So, my feeling is that those
> tokens are part of SaslHandshakeRequest and are just extended across
> multiple network packets. So modeling those as independent requests feels
> weird. When documentation them, we really need to document those as a
> sequence, not individual isolated requests that can be issued
> in arbitrary order. The version id will only add confusion since we can't
> version the tokens independently. We could explicitly add the client id and
> correlation id in the header, but I am not sure if it's worth the
> additional complexity.
>
> Thanks,
>
> Jun
>
> On Tue, Apr 12, 2016 at 1:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Jun,
> >
> > I understand the point about the SASL tokens being similar to the SSL
> > handshake in a way. However, is there any SASL library that handles the
> > network communication for these tokens? I couldn't find any and without
> > that, there isn't much benefit in deviating from Kafka's protocol (we
> > basically save the space taken by the request header). It's worth
> > mentioning that we are already adding the message size before the opaque
> > bytes provided by the library, so one could say we are already extending
> > the protocol.
> >
> > If we leave versioning aside, adding the standard Kafka request header to
> > those messages may also help from a debugging perspective as would then
> > include client id and correlation id along with the message.
> >
> > Ismael
> >
> > On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Magnus,
> > >
> > > That sounds reasonable. To reduce the changes on the server side, I'd
> > > suggest the following minor tweaks on the proposal.
> > >
> > > 1. Continue supporting the separate SASL and SASL_SSL port.
> > >
> > > On SASL port, we support the new sequence
> > > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> > > regular
> > > requests
> > >
> > > On SASL_SSL port, we support the new sequence
> > > SSL handshake bytes, ApiVersionRequest (optional),
> > > SaslHandshakeRequest,
> > > SASL tokens, regular requests
> > >
> > > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your
> argument
> > > about SSL handshake, those SASL tokens are generated by SASL library
> and
> > > Kafka doesn't really control its versioning. Kafka only controls the
> > > selection of SASL mechanism, which will be versioned in
> > > SaslHandshakeRequest.
> > >
> > > Does that work for you?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > >
> > > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> > > wrote:
> > >
> > > > Hey Jun, see inline
> > > >
> > > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > >
> > > > > Hi, Magnus,
> > > > >
> > > > > Let me understand your proposal in more details just from the
> > client's
> > > > > perspective. My understanding of your proposal is the following.
> > > > >
> > > > > On plaintext port, the client will send the following bytes in
> order.
> > > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL
> is
> > > > > enabled), regular requests
> > > > >
> > > > > On SSL port, the client will send the following bytes in order.
> > > > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest,
> > SASL
> > > > > tokens (if SASL is enabled), regular requests
> > > > >
> > > >
> > > >
> > > > Yup!
> > > > "SASL tokens" is a series of proper Kafka protocol
> > SaslHandshakeRequests
> > > > until
> > > > the handshake is done.
> > > >
> > > >
> > > >
> > > > >
> > > > > Is that right? Since we can use either SSL or SASL for
> > authentication,
> > > > it's
> > > > > weird that in one case, we require ApiVersionRequest to happen
> before
> > > > > authentication and in another case we require the reverse.
> > > > >
> > > >
> > > > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > > > libraries it
> > > > doesnt make sense to reimplement that on top of Kafka, so it isn't
> > really
> > > > comparable.
> > > > But for SASL there is no standardised handshake protocol so we must
> > > either
> > > > conceive one from scratch, or use the protocol that we already have
> > > > (Kafka).
> > > > For the initial SASL implementation in 0.9 the first option was
> chosen
> > > and
> > > > while
> > > > it required a new protocol implementation in supporting clients and
> the
> > > > broker
> > > > it served its purpose. But not for long, it already needs to evolve,
> > > > and this gives us a golden[1] opportunity to make the implementation
> > > > reusable, evolvable, less complex
> > > > and in line with all our other protocol work, by using the protocol
> > > stack
> > > > of Kafka which the
> > > > broker and all clients already have in place.
> > > >
> > > > Not taking this chance and instead diverging the custom SASL
> handshake
> > > > protocol
> > > > even further from Kafka seems to me a weird choice.
> > > >
> > > > The current KIP-43 proposal does not have a clear compatibility
> story;
> > it
> > > > doesnt seem to be possible
> > > > to upgrade clients before brokers, while this might be okay for the
> > Java
> > > > client, the KIP-35 discussion
> > > > has hopefully proven that this assumption can't be made for the
> entire
> > > > eco-system.
> > > >
> > > > Let me be clear that there isn't anything technically wrong with the
> > > KIP-43
> > > > proposal (well,
> > > > except for the hack to check byte[0] for 0x60 perhaps), but I'm
> worried
> > > the
> > > > proposal will eventually lead to
> > > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
> > handshake,
> > > > and this is just redundant,
> > > > there is no technical reason for doing so and it'll just make
> protocol
> > > > semantics and implementations more complex.
> > > >
> > > >
> > > > Regards,
> > > > Magnus
> > > >
> > > > [1]: Timing is good for this change since only two clients, Java and
> C,
> > > > currently supports
> > > > the existing SASL handshake so far.
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
> > magnus@edenhill.se>
> > > > > wrote:
> > > > >
> > > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > > > >
> > > > > > > Thinking about ApiVersionRequest a bit more. There are quite a
> > few
> > > > > things
> > > > > > > special about it. In the ideal case, (1) its version should
> never
> > > > > change;
> > > > > > >
> > > > > >
> > > > > > The only thing we know of the future is that we dont know
> anything,
> > > we
> > > > > > can't
> > > > > > think of every possible future use case, that's why need to be
> able
> > > to
> > > > > > evolve interfaces
> > > > > > as requirements and use-cases change. This is the gist of KIP-35,
> > and
> > > > > > hampering
> > > > > > KIP-35 itself, by not letting it also evolve, does not seem right
> > to
> > > me
> > > > > at
> > > > > > all.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > (2) it needs to be done before authentication (either
> SSL/SASL);
> > > (3)
> > > > it
> > > > > > is
> > > > > > > required to be issued at the beginning of each connection but
> > never
> > > > > needs
> > > > > > > to be issued again on the same connection. So, instead of
> > modeling
> > > it
> > > > > as
> > > > > > a
> > > > > > > regular request, it seems a cleaner approach is to just bake
> that
> > > > into
> > > > > > the
> > > > > > > initial connection handshake even before the authentication
> > layer.
> > > So
> > > > > the
> > > > > > > sequencing in a connection will be api discovery,
> authentication,
> > > > > > followed
> > > > > > > by regular requests. I am not sure we can still add this in a
> > > > backward
> > > > > > > compatible way now (e.g., not sure what the initial bytes from
> an
> > > SSL
> > > > > > > connection will look like). Even if we can do this in a
> backward
> > > > > > compatible
> > > > > > > way, it's probably non-trivial amount of work though.
> > > > > > >
> > > > > >
> > > > > > I have the luxory of not knowing the broker internals, so I can
> > only
> > > > > > discuss
> > > > > > this on a conceptual design level.
> > > > > >
> > > > > > In its simplest form each API request type has a NeedsAuth flag
> and
> > > the
> > > > > > broker protocol request layer simply checks if the current
> session
> > is
> > > > > > Authenticated
> > > > > > before processing the request: If not the session is closed and
> an
> > > > error
> > > > > is
> > > > > > logged.
> > > > > > The only two API requests that dont have the NeedsAuth flag would
> > be
> > > > > > SaslHandshakeRequest
> > > > > > and ApiVersionRequest, the latter could also use filtering to
> only
> > > > return
> > > > > > the same two
> > > > > > requests in ApiVersionResponse before the client is authenticated
> > (as
> > > > not
> > > > > > to "leak" information).
> > > > > > If authentication is not configured on the broker all sessions
> are
> > > > deemed
> > > > > > authenticated by default.
> > > > > >
> > > > > >
> > > > > > Re backwards compatibility:
> > > > > > My suggestion is to keep the current special SASL handshake
> > protocol
> > > on
> > > > > the
> > > > > > SASL_PLAIN/SASL_SSL
> > > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API
> > on
> > > > the
> > > > > > PLAIN/SSL endpoints.
> > > > > > This way the broker is backwards compatible with older clients
> that
> > > > only
> > > > > > supports the special SASL protocol,
> > > > > > and newer cliets are also backwards compatible with older brokers
> > > that
> > > > > only
> > > > > > supports the special SASL protocol.
> > > > > > Newer clients connecting to new brokers will be configured to use
> > > > > non-SASL
> > > > > > ports and use the
> > > > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > > > >
> > > > > > Using the existing standard Kafka protocol and the new
> future-proof
> > > > > > functionality of ApiVersionRequest
> > > > > > allows the in-band authentication mechanisms and semantics to
> > > naturally
> > > > > > evolve over time
> > > > > > without breaking existing clients.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > We started KIP-35 with supporting a client to know if a version
> > is
> > > > > > > supported by the broker. It's now evolved into supporting a
> > client
> > > to
> > > > > > > implement multiple versions of the protocol and dynamically
> pick
> > a
> > > > > > version
> > > > > > > supported by the broker. The former is likely solvable without
> > > > > > > ApiVersionRequest. How important is the latter? What if the C
> > > client
> > > > > just
> > > > > > > follows the java client model by implementing one version of
> > > protocol
> > > > > > per C
> > > > > > > client release (which seems easier to implement)?
> > > > > > >
> > > > > >
> > > > > > We've discussed this at length and it is not an option for
> > > librdkafka,
> > > > > nor
> > > > > > kafka-python, and
> > > > > > probably other clients as well, due to usability/UX and
> maintenance
> > > > > > reasons.
> > > > > > (There's even discussion of making the Java client more version
> > > > > agnostic!)
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
> > wrote:
> > > > > > >
> > > > > > > > Magnus,
> > > > > > > >
> > > > > > > > A while back, we had another proposal for the broker to just
> > send
> > > > the
> > > > > > > > correlation id and an empty payload if it receives an
> > unsupported
> > > > > > version
> > > > > > > > of the request. I didn't see that in the rejected section. It
> > > seems
> > > > > > > simpler
> > > > > > > > than the current proposal where the client has to issue an
> > > > > > > > ApiVersionRequest on every connection. Could you document the
> > > > reason
> > > > > > why
> > > > > > > we
> > > > > > > > rejected it?
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> > > asingh@cloudera.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > > >>
> > > > > > > >> > Two more things:
> > > > > > > >> >
> > > > > > > >> > 3. We talk about backporting of new request versions to
> > stable
> > > > > > > branches
> > > > > > > >> in
> > > > > > > >> > the KIP. In practice, we can't do that until the Java
> client
> > > is
> > > > > > > changed
> > > > > > > >> so
> > > > > > > >> > that it doesn't blindly use the latest protocol version.
> > > > > Otherwise,
> > > > > > if
> > > > > > > >> new
> > > > > > > >> > request versions were added to 0.9.0.2, the client would
> > break
> > > > > when
> > > > > > > >> talking
> > > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would
> fail a
> > > bit
> > > > > > more
> > > > > > > >> > gracefully, but that's still not good enough for a stable
> > > > branch).
> > > > > > It
> > > > > > > >> may
> > > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > > > orthogonal
> > > > > > and
> > > > > > > >> > doesn't prevent the KIP from being adopted, but good to
> > avoid
> > > > > > > >> confusion).
> > > > > > > >> >
> > > > > > > >> Good point. Adding this note and also adding a note that
> Kafka
> > > has
> > > > > not
> > > > > > > >> backported an api version so far.
> > > > > > > >>
> > > > > > > >> >
> > > > > > > >> > 4. The paragraph below is a bit confusing. It starts
> talking
> > > > about
> > > > > > > 0.9.0
> > > > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > > > >> >
> > > > > > > >> Yes.
> > > > > > > >>
> > > > > > > >> >
> > > > > > > >> > "Deprecation of a protocol version will be done by
> marking a
> > > > > > protocol
> > > > > > > >> > version as deprecated in protocol documentation.
> > Documentation
> > > > > shall
> > > > > > > >> also
> > > > > > > >> > be used to indicate a protocol version that must not be
> > used,
> > > or
> > > > > for
> > > > > > > any
> > > > > > > >> > such information.For instance, say 0.9.0 had protocol
> > versions
> > > > [0]
> > > > > > for
> > > > > > > >> api
> > > > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > > > running
> > > > > > off
> > > > > > > >> > trunk started using version 1 of the api and found out a
> > major
> > > > > bug.
> > > > > > To
> > > > > > > >> > rectify that version 2 of the api is added to trunk. For
> > some
> > > > > > reason,
> > > > > > > >> it is
> > > > > > > >> > now deemed important to have version 2 of the api in 0.9.1
> > as
> > > > > well.
> > > > > > To
> > > > > > > >> do
> > > > > > > >> > so, version 1 and version 2 both of the api will be
> > backported
> > > > to
> > > > > > the
> > > > > > > >> 0.9.1
> > > > > > > >> > branch. 0.9.1 broker will return 0 as min supported
> version
> > > for
> > > > > the
> > > > > > > api
> > > > > > > >> and
> > > > > > > >> > 2 for the max supported version for the api. However, the
> > > > version
> > > > > 1
> > > > > > > >> should
> > > > > > > >> > be clearly marked as deprecated on its documentation. It
> > will
> > > be
> > > > > > > >> client's
> > > > > > > >> > responsibility to make sure they are not using any such
> > > > deprecated
> > > > > > > >> version
> > > > > > > >> > to the best knowledge of the client at the time of
> > development
> > > > (or
> > > > > > > >> > alternatively by configuration)."
> > > > > > > >> >
> > > > > > > >> > Ismael
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> > > ismael@juma.me.uk>
> > > > > > > wrote:
> > > > > > > >> >
> > > > > > > >> > > A couple of questions:
> > > > > > > >> > >
> > > > > > > >> > > 1. The KIP says "Specific version may be deprecated
> > through
> > > > > > protocol
> > > > > > > >> > > documentation but must still be supported (although it
> is
> > > fair
> > > > > to
> > > > > > > >> return
> > > > > > > >> > an
> > > > > > > >> > > error code if the specific API supports it).". It may be
> > > worth
> > > > > > > >> expanding
> > > > > > > >> > > this a little more. For example, what does it mean to
> > > support
> > > > > the
> > > > > > > >> API? I
> > > > > > > >> > > guess this means that the broker must not disconnect the
> > > > client
> > > > > > and
> > > > > > > >> the
> > > > > > > >> > > broker must return a valid protocol response. Given that
> > it
> > > > says
> > > > > > > that
> > > > > > > >> it
> > > > > > > >> > is
> > > > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > > > return
> > > > > an
> > > > > > > >> error
> > > > > > > >> > > code if the specific API supports it, it sounds like we
> > are
> > > > > saying
> > > > > > > >> that
> > > > > > > >> > we
> > > > > > > >> > > don't have to maintain the semantic behaviour (i.e. we
> > could
> > > > > > > _always_
> > > > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > > > >> > >
> > > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > > > >> ApiVersionRequest?
> > > > > > > >> > >
> > > > > > > >> > > Ismael
> > > > > > > >> > >
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> --
> > > > > > > >>
> > > > > > > >> Regards,
> > > > > > > >> Ashish
> > > > > > > >>
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
Regards,
Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Jun Rao <ju...@confluent.io>.
Hi, Ismael,
The SASL engine that we used is the SASL library, right? How did the C
client generate those SASL tokens? Once a SASL mechanism is chosen, the
subsequent tokens are determined, right? So, my feeling is that those
tokens are part of SaslHandshakeRequest and are just extended across
multiple network packets. So modeling those as independent requests feels
weird. When documentation them, we really need to document those as a
sequence, not individual isolated requests that can be issued
in arbitrary order. The version id will only add confusion since we can't
version the tokens independently. We could explicitly add the client id and
correlation id in the header, but I am not sure if it's worth the
additional complexity.
Thanks,
Jun
On Tue, Apr 12, 2016 at 1:18 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Hi Jun,
>
> I understand the point about the SASL tokens being similar to the SSL
> handshake in a way. However, is there any SASL library that handles the
> network communication for these tokens? I couldn't find any and without
> that, there isn't much benefit in deviating from Kafka's protocol (we
> basically save the space taken by the request header). It's worth
> mentioning that we are already adding the message size before the opaque
> bytes provided by the library, so one could say we are already extending
> the protocol.
>
> If we leave versioning aside, adding the standard Kafka request header to
> those messages may also help from a debugging perspective as would then
> include client id and correlation id along with the message.
>
> Ismael
>
> On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
>
> > Magnus,
> >
> > That sounds reasonable. To reduce the changes on the server side, I'd
> > suggest the following minor tweaks on the proposal.
> >
> > 1. Continue supporting the separate SASL and SASL_SSL port.
> >
> > On SASL port, we support the new sequence
> > ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> > regular
> > requests
> >
> > On SASL_SSL port, we support the new sequence
> > SSL handshake bytes, ApiVersionRequest (optional),
> > SaslHandshakeRequest,
> > SASL tokens, regular requests
> >
> > 2. We don't wrap SASL tokens in Kafka protocol. Similar to your argument
> > about SSL handshake, those SASL tokens are generated by SASL library and
> > Kafka doesn't really control its versioning. Kafka only controls the
> > selection of SASL mechanism, which will be versioned in
> > SaslHandshakeRequest.
> >
> > Does that work for you?
> >
> > Thanks,
> >
> > Jun
> >
> >
> > On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> > wrote:
> >
> > > Hey Jun, see inline
> > >
> > > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > >
> > > > Hi, Magnus,
> > > >
> > > > Let me understand your proposal in more details just from the
> client's
> > > > perspective. My understanding of your proposal is the following.
> > > >
> > > > On plaintext port, the client will send the following bytes in order.
> > > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> > > > enabled), regular requests
> > > >
> > > > On SSL port, the client will send the following bytes in order.
> > > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest,
> SASL
> > > > tokens (if SASL is enabled), regular requests
> > > >
> > >
> > >
> > > Yup!
> > > "SASL tokens" is a series of proper Kafka protocol
> SaslHandshakeRequests
> > > until
> > > the handshake is done.
> > >
> > >
> > >
> > > >
> > > > Is that right? Since we can use either SSL or SASL for
> authentication,
> > > it's
> > > > weird that in one case, we require ApiVersionRequest to happen before
> > > > authentication and in another case we require the reverse.
> > > >
> > >
> > > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > > libraries it
> > > doesnt make sense to reimplement that on top of Kafka, so it isn't
> really
> > > comparable.
> > > But for SASL there is no standardised handshake protocol so we must
> > either
> > > conceive one from scratch, or use the protocol that we already have
> > > (Kafka).
> > > For the initial SASL implementation in 0.9 the first option was chosen
> > and
> > > while
> > > it required a new protocol implementation in supporting clients and the
> > > broker
> > > it served its purpose. But not for long, it already needs to evolve,
> > > and this gives us a golden[1] opportunity to make the implementation
> > > reusable, evolvable, less complex
> > > and in line with all our other protocol work, by using the protocol
> > stack
> > > of Kafka which the
> > > broker and all clients already have in place.
> > >
> > > Not taking this chance and instead diverging the custom SASL handshake
> > > protocol
> > > even further from Kafka seems to me a weird choice.
> > >
> > > The current KIP-43 proposal does not have a clear compatibility story;
> it
> > > doesnt seem to be possible
> > > to upgrade clients before brokers, while this might be okay for the
> Java
> > > client, the KIP-35 discussion
> > > has hopefully proven that this assumption can't be made for the entire
> > > eco-system.
> > >
> > > Let me be clear that there isn't anything technically wrong with the
> > KIP-43
> > > proposal (well,
> > > except for the hack to check byte[0] for 0x60 perhaps), but I'm worried
> > the
> > > proposal will eventually lead to
> > > reimplementing Api Versioning, KIP-35, etc, in the custom SASL
> handshake,
> > > and this is just redundant,
> > > there is no technical reason for doing so and it'll just make protocol
> > > semantics and implementations more complex.
> > >
> > >
> > > Regards,
> > > Magnus
> > >
> > > [1]: Timing is good for this change since only two clients, Java and C,
> > > currently supports
> > > the existing SASL handshake so far.
> > >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <
> magnus@edenhill.se>
> > > > wrote:
> > > >
> > > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > > >
> > > > > > Thinking about ApiVersionRequest a bit more. There are quite a
> few
> > > > things
> > > > > > special about it. In the ideal case, (1) its version should never
> > > > change;
> > > > > >
> > > > >
> > > > > The only thing we know of the future is that we dont know anything,
> > we
> > > > > can't
> > > > > think of every possible future use case, that's why need to be able
> > to
> > > > > evolve interfaces
> > > > > as requirements and use-cases change. This is the gist of KIP-35,
> and
> > > > > hampering
> > > > > KIP-35 itself, by not letting it also evolve, does not seem right
> to
> > me
> > > > at
> > > > > all.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > (2) it needs to be done before authentication (either SSL/SASL);
> > (3)
> > > it
> > > > > is
> > > > > > required to be issued at the beginning of each connection but
> never
> > > > needs
> > > > > > to be issued again on the same connection. So, instead of
> modeling
> > it
> > > > as
> > > > > a
> > > > > > regular request, it seems a cleaner approach is to just bake that
> > > into
> > > > > the
> > > > > > initial connection handshake even before the authentication
> layer.
> > So
> > > > the
> > > > > > sequencing in a connection will be api discovery, authentication,
> > > > > followed
> > > > > > by regular requests. I am not sure we can still add this in a
> > > backward
> > > > > > compatible way now (e.g., not sure what the initial bytes from an
> > SSL
> > > > > > connection will look like). Even if we can do this in a backward
> > > > > compatible
> > > > > > way, it's probably non-trivial amount of work though.
> > > > > >
> > > > >
> > > > > I have the luxory of not knowing the broker internals, so I can
> only
> > > > > discuss
> > > > > this on a conceptual design level.
> > > > >
> > > > > In its simplest form each API request type has a NeedsAuth flag and
> > the
> > > > > broker protocol request layer simply checks if the current session
> is
> > > > > Authenticated
> > > > > before processing the request: If not the session is closed and an
> > > error
> > > > is
> > > > > logged.
> > > > > The only two API requests that dont have the NeedsAuth flag would
> be
> > > > > SaslHandshakeRequest
> > > > > and ApiVersionRequest, the latter could also use filtering to only
> > > return
> > > > > the same two
> > > > > requests in ApiVersionResponse before the client is authenticated
> (as
> > > not
> > > > > to "leak" information).
> > > > > If authentication is not configured on the broker all sessions are
> > > deemed
> > > > > authenticated by default.
> > > > >
> > > > >
> > > > > Re backwards compatibility:
> > > > > My suggestion is to keep the current special SASL handshake
> protocol
> > on
> > > > the
> > > > > SASL_PLAIN/SASL_SSL
> > > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API
> on
> > > the
> > > > > PLAIN/SSL endpoints.
> > > > > This way the broker is backwards compatible with older clients that
> > > only
> > > > > supports the special SASL protocol,
> > > > > and newer cliets are also backwards compatible with older brokers
> > that
> > > > only
> > > > > supports the special SASL protocol.
> > > > > Newer clients connecting to new brokers will be configured to use
> > > > non-SASL
> > > > > ports and use the
> > > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > > >
> > > > > Using the existing standard Kafka protocol and the new future-proof
> > > > > functionality of ApiVersionRequest
> > > > > allows the in-band authentication mechanisms and semantics to
> > naturally
> > > > > evolve over time
> > > > > without breaking existing clients.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > We started KIP-35 with supporting a client to know if a version
> is
> > > > > > supported by the broker. It's now evolved into supporting a
> client
> > to
> > > > > > implement multiple versions of the protocol and dynamically pick
> a
> > > > > version
> > > > > > supported by the broker. The former is likely solvable without
> > > > > > ApiVersionRequest. How important is the latter? What if the C
> > client
> > > > just
> > > > > > follows the java client model by implementing one version of
> > protocol
> > > > > per C
> > > > > > client release (which seems easier to implement)?
> > > > > >
> > > > >
> > > > > We've discussed this at length and it is not an option for
> > librdkafka,
> > > > nor
> > > > > kafka-python, and
> > > > > probably other clients as well, due to usability/UX and maintenance
> > > > > reasons.
> > > > > (There's even discussion of making the Java client more version
> > > > agnostic!)
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > >
> > > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io>
> wrote:
> > > > > >
> > > > > > > Magnus,
> > > > > > >
> > > > > > > A while back, we had another proposal for the broker to just
> send
> > > the
> > > > > > > correlation id and an empty payload if it receives an
> unsupported
> > > > > version
> > > > > > > of the request. I didn't see that in the rejected section. It
> > seems
> > > > > > simpler
> > > > > > > than the current proposal where the client has to issue an
> > > > > > > ApiVersionRequest on every connection. Could you document the
> > > reason
> > > > > why
> > > > > > we
> > > > > > > rejected it?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> > asingh@cloudera.com>
> > > > > > wrote:
> > > > > > >
> > > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > > >>
> > > > > > >> > Two more things:
> > > > > > >> >
> > > > > > >> > 3. We talk about backporting of new request versions to
> stable
> > > > > > branches
> > > > > > >> in
> > > > > > >> > the KIP. In practice, we can't do that until the Java client
> > is
> > > > > > changed
> > > > > > >> so
> > > > > > >> > that it doesn't blindly use the latest protocol version.
> > > > Otherwise,
> > > > > if
> > > > > > >> new
> > > > > > >> > request versions were added to 0.9.0.2, the client would
> break
> > > > when
> > > > > > >> talking
> > > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a
> > bit
> > > > > more
> > > > > > >> > gracefully, but that's still not good enough for a stable
> > > branch).
> > > > > It
> > > > > > >> may
> > > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > > orthogonal
> > > > > and
> > > > > > >> > doesn't prevent the KIP from being adopted, but good to
> avoid
> > > > > > >> confusion).
> > > > > > >> >
> > > > > > >> Good point. Adding this note and also adding a note that Kafka
> > has
> > > > not
> > > > > > >> backported an api version so far.
> > > > > > >>
> > > > > > >> >
> > > > > > >> > 4. The paragraph below is a bit confusing. It starts talking
> > > about
> > > > > > 0.9.0
> > > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > > >> >
> > > > > > >> Yes.
> > > > > > >>
> > > > > > >> >
> > > > > > >> > "Deprecation of a protocol version will be done by marking a
> > > > > protocol
> > > > > > >> > version as deprecated in protocol documentation.
> Documentation
> > > > shall
> > > > > > >> also
> > > > > > >> > be used to indicate a protocol version that must not be
> used,
> > or
> > > > for
> > > > > > any
> > > > > > >> > such information.For instance, say 0.9.0 had protocol
> versions
> > > [0]
> > > > > for
> > > > > > >> api
> > > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > > running
> > > > > off
> > > > > > >> > trunk started using version 1 of the api and found out a
> major
> > > > bug.
> > > > > To
> > > > > > >> > rectify that version 2 of the api is added to trunk. For
> some
> > > > > reason,
> > > > > > >> it is
> > > > > > >> > now deemed important to have version 2 of the api in 0.9.1
> as
> > > > well.
> > > > > To
> > > > > > >> do
> > > > > > >> > so, version 1 and version 2 both of the api will be
> backported
> > > to
> > > > > the
> > > > > > >> 0.9.1
> > > > > > >> > branch. 0.9.1 broker will return 0 as min supported version
> > for
> > > > the
> > > > > > api
> > > > > > >> and
> > > > > > >> > 2 for the max supported version for the api. However, the
> > > version
> > > > 1
> > > > > > >> should
> > > > > > >> > be clearly marked as deprecated on its documentation. It
> will
> > be
> > > > > > >> client's
> > > > > > >> > responsibility to make sure they are not using any such
> > > deprecated
> > > > > > >> version
> > > > > > >> > to the best knowledge of the client at the time of
> development
> > > (or
> > > > > > >> > alternatively by configuration)."
> > > > > > >> >
> > > > > > >> > Ismael
> > > > > > >> >
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > >> >
> > > > > > >> > > A couple of questions:
> > > > > > >> > >
> > > > > > >> > > 1. The KIP says "Specific version may be deprecated
> through
> > > > > protocol
> > > > > > >> > > documentation but must still be supported (although it is
> > fair
> > > > to
> > > > > > >> return
> > > > > > >> > an
> > > > > > >> > > error code if the specific API supports it).". It may be
> > worth
> > > > > > >> expanding
> > > > > > >> > > this a little more. For example, what does it mean to
> > support
> > > > the
> > > > > > >> API? I
> > > > > > >> > > guess this means that the broker must not disconnect the
> > > client
> > > > > and
> > > > > > >> the
> > > > > > >> > > broker must return a valid protocol response. Given that
> it
> > > says
> > > > > > that
> > > > > > >> it
> > > > > > >> > is
> > > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > > return
> > > > an
> > > > > > >> error
> > > > > > >> > > code if the specific API supports it, it sounds like we
> are
> > > > saying
> > > > > > >> that
> > > > > > >> > we
> > > > > > >> > > don't have to maintain the semantic behaviour (i.e. we
> could
> > > > > > _always_
> > > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > > >> > >
> > > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > > >> ApiVersionRequest?
> > > > > > >> > >
> > > > > > >> > > Ismael
> > > > > > >> > >
> > > > > > >> >
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > > >> --
> > > > > > >>
> > > > > > >> Regards,
> > > > > > >> Ashish
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Jun,
I understand the point about the SASL tokens being similar to the SSL
handshake in a way. However, is there any SASL library that handles the
network communication for these tokens? I couldn't find any and without
that, there isn't much benefit in deviating from Kafka's protocol (we
basically save the space taken by the request header). It's worth
mentioning that we are already adding the message size before the opaque
bytes provided by the library, so one could say we are already extending
the protocol.
If we leave versioning aside, adding the standard Kafka request header to
those messages may also help from a debugging perspective as would then
include client id and correlation id along with the message.
Ismael
On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
> Magnus,
>
> That sounds reasonable. To reduce the changes on the server side, I'd
> suggest the following minor tweaks on the proposal.
>
> 1. Continue supporting the separate SASL and SASL_SSL port.
>
> On SASL port, we support the new sequence
> ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> regular
> requests
>
> On SASL_SSL port, we support the new sequence
> SSL handshake bytes, ApiVersionRequest (optional),
> SaslHandshakeRequest,
> SASL tokens, regular requests
>
> 2. We don't wrap SASL tokens in Kafka protocol. Similar to your argument
> about SSL handshake, those SASL tokens are generated by SASL library and
> Kafka doesn't really control its versioning. Kafka only controls the
> selection of SASL mechanism, which will be versioned in
> SaslHandshakeRequest.
>
> Does that work for you?
>
> Thanks,
>
> Jun
>
>
> On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> wrote:
>
> > Hey Jun, see inline
> >
> > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> >
> > > Hi, Magnus,
> > >
> > > Let me understand your proposal in more details just from the client's
> > > perspective. My understanding of your proposal is the following.
> > >
> > > On plaintext port, the client will send the following bytes in order.
> > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> > > enabled), regular requests
> > >
> > > On SSL port, the client will send the following bytes in order.
> > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest, SASL
> > > tokens (if SASL is enabled), regular requests
> > >
> >
> >
> > Yup!
> > "SASL tokens" is a series of proper Kafka protocol SaslHandshakeRequests
> > until
> > the handshake is done.
> >
> >
> >
> > >
> > > Is that right? Since we can use either SSL or SASL for authentication,
> > it's
> > > weird that in one case, we require ApiVersionRequest to happen before
> > > authentication and in another case we require the reverse.
> > >
> >
> > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > libraries it
> > doesnt make sense to reimplement that on top of Kafka, so it isn't really
> > comparable.
> > But for SASL there is no standardised handshake protocol so we must
> either
> > conceive one from scratch, or use the protocol that we already have
> > (Kafka).
> > For the initial SASL implementation in 0.9 the first option was chosen
> and
> > while
> > it required a new protocol implementation in supporting clients and the
> > broker
> > it served its purpose. But not for long, it already needs to evolve,
> > and this gives us a golden[1] opportunity to make the implementation
> > reusable, evolvable, less complex
> > and in line with all our other protocol work, by using the protocol
> stack
> > of Kafka which the
> > broker and all clients already have in place.
> >
> > Not taking this chance and instead diverging the custom SASL handshake
> > protocol
> > even further from Kafka seems to me a weird choice.
> >
> > The current KIP-43 proposal does not have a clear compatibility story; it
> > doesnt seem to be possible
> > to upgrade clients before brokers, while this might be okay for the Java
> > client, the KIP-35 discussion
> > has hopefully proven that this assumption can't be made for the entire
> > eco-system.
> >
> > Let me be clear that there isn't anything technically wrong with the
> KIP-43
> > proposal (well,
> > except for the hack to check byte[0] for 0x60 perhaps), but I'm worried
> the
> > proposal will eventually lead to
> > reimplementing Api Versioning, KIP-35, etc, in the custom SASL handshake,
> > and this is just redundant,
> > there is no technical reason for doing so and it'll just make protocol
> > semantics and implementations more complex.
> >
> >
> > Regards,
> > Magnus
> >
> > [1]: Timing is good for this change since only two clients, Java and C,
> > currently supports
> > the existing SASL handshake so far.
> >
> >
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <ma...@edenhill.se>
> > > wrote:
> > >
> > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > >
> > > > > Thinking about ApiVersionRequest a bit more. There are quite a few
> > > things
> > > > > special about it. In the ideal case, (1) its version should never
> > > change;
> > > > >
> > > >
> > > > The only thing we know of the future is that we dont know anything,
> we
> > > > can't
> > > > think of every possible future use case, that's why need to be able
> to
> > > > evolve interfaces
> > > > as requirements and use-cases change. This is the gist of KIP-35, and
> > > > hampering
> > > > KIP-35 itself, by not letting it also evolve, does not seem right to
> me
> > > at
> > > > all.
> > > >
> > > >
> > > >
> > > >
> > > > > (2) it needs to be done before authentication (either SSL/SASL);
> (3)
> > it
> > > > is
> > > > > required to be issued at the beginning of each connection but never
> > > needs
> > > > > to be issued again on the same connection. So, instead of modeling
> it
> > > as
> > > > a
> > > > > regular request, it seems a cleaner approach is to just bake that
> > into
> > > > the
> > > > > initial connection handshake even before the authentication layer.
> So
> > > the
> > > > > sequencing in a connection will be api discovery, authentication,
> > > > followed
> > > > > by regular requests. I am not sure we can still add this in a
> > backward
> > > > > compatible way now (e.g., not sure what the initial bytes from an
> SSL
> > > > > connection will look like). Even if we can do this in a backward
> > > > compatible
> > > > > way, it's probably non-trivial amount of work though.
> > > > >
> > > >
> > > > I have the luxory of not knowing the broker internals, so I can only
> > > > discuss
> > > > this on a conceptual design level.
> > > >
> > > > In its simplest form each API request type has a NeedsAuth flag and
> the
> > > > broker protocol request layer simply checks if the current session is
> > > > Authenticated
> > > > before processing the request: If not the session is closed and an
> > error
> > > is
> > > > logged.
> > > > The only two API requests that dont have the NeedsAuth flag would be
> > > > SaslHandshakeRequest
> > > > and ApiVersionRequest, the latter could also use filtering to only
> > return
> > > > the same two
> > > > requests in ApiVersionResponse before the client is authenticated (as
> > not
> > > > to "leak" information).
> > > > If authentication is not configured on the broker all sessions are
> > deemed
> > > > authenticated by default.
> > > >
> > > >
> > > > Re backwards compatibility:
> > > > My suggestion is to keep the current special SASL handshake protocol
> on
> > > the
> > > > SASL_PLAIN/SASL_SSL
> > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API on
> > the
> > > > PLAIN/SSL endpoints.
> > > > This way the broker is backwards compatible with older clients that
> > only
> > > > supports the special SASL protocol,
> > > > and newer cliets are also backwards compatible with older brokers
> that
> > > only
> > > > supports the special SASL protocol.
> > > > Newer clients connecting to new brokers will be configured to use
> > > non-SASL
> > > > ports and use the
> > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > >
> > > > Using the existing standard Kafka protocol and the new future-proof
> > > > functionality of ApiVersionRequest
> > > > allows the in-band authentication mechanisms and semantics to
> naturally
> > > > evolve over time
> > > > without breaking existing clients.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > We started KIP-35 with supporting a client to know if a version is
> > > > > supported by the broker. It's now evolved into supporting a client
> to
> > > > > implement multiple versions of the protocol and dynamically pick a
> > > > version
> > > > > supported by the broker. The former is likely solvable without
> > > > > ApiVersionRequest. How important is the latter? What if the C
> client
> > > just
> > > > > follows the java client model by implementing one version of
> protocol
> > > > per C
> > > > > client release (which seems easier to implement)?
> > > > >
> > > >
> > > > We've discussed this at length and it is not an option for
> librdkafka,
> > > nor
> > > > kafka-python, and
> > > > probably other clients as well, due to usability/UX and maintenance
> > > > reasons.
> > > > (There's even discussion of making the Java client more version
> > > agnostic!)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > >
> > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> > > > >
> > > > > > Magnus,
> > > > > >
> > > > > > A while back, we had another proposal for the broker to just send
> > the
> > > > > > correlation id and an empty payload if it receives an unsupported
> > > > version
> > > > > > of the request. I didn't see that in the rejected section. It
> seems
> > > > > simpler
> > > > > > than the current proposal where the client has to issue an
> > > > > > ApiVersionRequest on every connection. Could you document the
> > reason
> > > > why
> > > > > we
> > > > > > rejected it?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> asingh@cloudera.com>
> > > > > wrote:
> > > > > >
> > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk>
> > > > wrote:
> > > > > >>
> > > > > >> > Two more things:
> > > > > >> >
> > > > > >> > 3. We talk about backporting of new request versions to stable
> > > > > branches
> > > > > >> in
> > > > > >> > the KIP. In practice, we can't do that until the Java client
> is
> > > > > changed
> > > > > >> so
> > > > > >> > that it doesn't blindly use the latest protocol version.
> > > Otherwise,
> > > > if
> > > > > >> new
> > > > > >> > request versions were added to 0.9.0.2, the client would break
> > > when
> > > > > >> talking
> > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a
> bit
> > > > more
> > > > > >> > gracefully, but that's still not good enough for a stable
> > branch).
> > > > It
> > > > > >> may
> > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > orthogonal
> > > > and
> > > > > >> > doesn't prevent the KIP from being adopted, but good to avoid
> > > > > >> confusion).
> > > > > >> >
> > > > > >> Good point. Adding this note and also adding a note that Kafka
> has
> > > not
> > > > > >> backported an api version so far.
> > > > > >>
> > > > > >> >
> > > > > >> > 4. The paragraph below is a bit confusing. It starts talking
> > about
> > > > > 0.9.0
> > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > >> >
> > > > > >> Yes.
> > > > > >>
> > > > > >> >
> > > > > >> > "Deprecation of a protocol version will be done by marking a
> > > > protocol
> > > > > >> > version as deprecated in protocol documentation. Documentation
> > > shall
> > > > > >> also
> > > > > >> > be used to indicate a protocol version that must not be used,
> or
> > > for
> > > > > any
> > > > > >> > such information.For instance, say 0.9.0 had protocol versions
> > [0]
> > > > for
> > > > > >> api
> > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > running
> > > > off
> > > > > >> > trunk started using version 1 of the api and found out a major
> > > bug.
> > > > To
> > > > > >> > rectify that version 2 of the api is added to trunk. For some
> > > > reason,
> > > > > >> it is
> > > > > >> > now deemed important to have version 2 of the api in 0.9.1 as
> > > well.
> > > > To
> > > > > >> do
> > > > > >> > so, version 1 and version 2 both of the api will be backported
> > to
> > > > the
> > > > > >> 0.9.1
> > > > > >> > branch. 0.9.1 broker will return 0 as min supported version
> for
> > > the
> > > > > api
> > > > > >> and
> > > > > >> > 2 for the max supported version for the api. However, the
> > version
> > > 1
> > > > > >> should
> > > > > >> > be clearly marked as deprecated on its documentation. It will
> be
> > > > > >> client's
> > > > > >> > responsibility to make sure they are not using any such
> > deprecated
> > > > > >> version
> > > > > >> > to the best knowledge of the client at the time of development
> > (or
> > > > > >> > alternatively by configuration)."
> > > > > >> >
> > > > > >> > Ismael
> > > > > >> >
> > > > > >> >
> > > > > >> >
> > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > >> >
> > > > > >> > > A couple of questions:
> > > > > >> > >
> > > > > >> > > 1. The KIP says "Specific version may be deprecated through
> > > > protocol
> > > > > >> > > documentation but must still be supported (although it is
> fair
> > > to
> > > > > >> return
> > > > > >> > an
> > > > > >> > > error code if the specific API supports it).". It may be
> worth
> > > > > >> expanding
> > > > > >> > > this a little more. For example, what does it mean to
> support
> > > the
> > > > > >> API? I
> > > > > >> > > guess this means that the broker must not disconnect the
> > client
> > > > and
> > > > > >> the
> > > > > >> > > broker must return a valid protocol response. Given that it
> > says
> > > > > that
> > > > > >> it
> > > > > >> > is
> > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > return
> > > an
> > > > > >> error
> > > > > >> > > code if the specific API supports it, it sounds like we are
> > > saying
> > > > > >> that
> > > > > >> > we
> > > > > >> > > don't have to maintain the semantic behaviour (i.e. we could
> > > > > _always_
> > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > >> > >
> > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > >> ApiVersionRequest?
> > > > > >> > >
> > > > > >> > > Ismael
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> --
> > > > > >>
> > > > > >> Regards,
> > > > > >> Ashish
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Rajini Sivaram <ra...@googlemail.com>.
Jun's tweaked proposal sounds good to me. In terms of completing KIP-43,
this changes the format of the request-response for exchanging mechanisms,
but not the overall logic. Since the request format in KIP-43 is worth
changing anyway, I will update the KIP and the PR.
On Tue, Apr 12, 2016 at 2:13 AM, Jun Rao <ju...@confluent.io> wrote:
> Magnus,
>
> That sounds reasonable. To reduce the changes on the server side, I'd
> suggest the following minor tweaks on the proposal.
>
> 1. Continue supporting the separate SASL and SASL_SSL port.
>
> On SASL port, we support the new sequence
> ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens,
> regular
> requests
>
> On SASL_SSL port, we support the new sequence
> SSL handshake bytes, ApiVersionRequest (optional),
> SaslHandshakeRequest,
> SASL tokens, regular requests
>
> 2. We don't wrap SASL tokens in Kafka protocol. Similar to your argument
> about SSL handshake, those SASL tokens are generated by SASL library and
> Kafka doesn't really control its versioning. Kafka only controls the
> selection of SASL mechanism, which will be versioned in
> SaslHandshakeRequest.
>
> Does that work for you?
>
> Thanks,
>
> Jun
>
>
> On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
> wrote:
>
> > Hey Jun, see inline
> >
> > 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> >
> > > Hi, Magnus,
> > >
> > > Let me understand your proposal in more details just from the client's
> > > perspective. My understanding of your proposal is the following.
> > >
> > > On plaintext port, the client will send the following bytes in order.
> > > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> > > enabled), regular requests
> > >
> > > On SSL port, the client will send the following bytes in order.
> > > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest, SASL
> > > tokens (if SASL is enabled), regular requests
> > >
> >
> >
> > Yup!
> > "SASL tokens" is a series of proper Kafka protocol SaslHandshakeRequests
> > until
> > the handshake is done.
> >
> >
> >
> > >
> > > Is that right? Since we can use either SSL or SASL for authentication,
> > it's
> > > weird that in one case, we require ApiVersionRequest to happen before
> > > authentication and in another case we require the reverse.
> > >
> >
> > Since the SSL/TLS is standardised and taken care of for us by the SSL
> > libraries it
> > doesnt make sense to reimplement that on top of Kafka, so it isn't really
> > comparable.
> > But for SASL there is no standardised handshake protocol so we must
> either
> > conceive one from scratch, or use the protocol that we already have
> > (Kafka).
> > For the initial SASL implementation in 0.9 the first option was chosen
> and
> > while
> > it required a new protocol implementation in supporting clients and the
> > broker
> > it served its purpose. But not for long, it already needs to evolve,
> > and this gives us a golden[1] opportunity to make the implementation
> > reusable, evolvable, less complex
> > and in line with all our other protocol work, by using the protocol
> stack
> > of Kafka which the
> > broker and all clients already have in place.
> >
> > Not taking this chance and instead diverging the custom SASL handshake
> > protocol
> > even further from Kafka seems to me a weird choice.
> >
> > The current KIP-43 proposal does not have a clear compatibility story; it
> > doesnt seem to be possible
> > to upgrade clients before brokers, while this might be okay for the Java
> > client, the KIP-35 discussion
> > has hopefully proven that this assumption can't be made for the entire
> > eco-system.
> >
> > Let me be clear that there isn't anything technically wrong with the
> KIP-43
> > proposal (well,
> > except for the hack to check byte[0] for 0x60 perhaps), but I'm worried
> the
> > proposal will eventually lead to
> > reimplementing Api Versioning, KIP-35, etc, in the custom SASL handshake,
> > and this is just redundant,
> > there is no technical reason for doing so and it'll just make protocol
> > semantics and implementations more complex.
> >
> >
> > Regards,
> > Magnus
> >
> > [1]: Timing is good for this change since only two clients, Java and C,
> > currently supports
> > the existing SASL handshake so far.
> >
> >
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <ma...@edenhill.se>
> > > wrote:
> > >
> > > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > > >
> > > > > Thinking about ApiVersionRequest a bit more. There are quite a few
> > > things
> > > > > special about it. In the ideal case, (1) its version should never
> > > change;
> > > > >
> > > >
> > > > The only thing we know of the future is that we dont know anything,
> we
> > > > can't
> > > > think of every possible future use case, that's why need to be able
> to
> > > > evolve interfaces
> > > > as requirements and use-cases change. This is the gist of KIP-35, and
> > > > hampering
> > > > KIP-35 itself, by not letting it also evolve, does not seem right to
> me
> > > at
> > > > all.
> > > >
> > > >
> > > >
> > > >
> > > > > (2) it needs to be done before authentication (either SSL/SASL);
> (3)
> > it
> > > > is
> > > > > required to be issued at the beginning of each connection but never
> > > needs
> > > > > to be issued again on the same connection. So, instead of modeling
> it
> > > as
> > > > a
> > > > > regular request, it seems a cleaner approach is to just bake that
> > into
> > > > the
> > > > > initial connection handshake even before the authentication layer.
> So
> > > the
> > > > > sequencing in a connection will be api discovery, authentication,
> > > > followed
> > > > > by regular requests. I am not sure we can still add this in a
> > backward
> > > > > compatible way now (e.g., not sure what the initial bytes from an
> SSL
> > > > > connection will look like). Even if we can do this in a backward
> > > > compatible
> > > > > way, it's probably non-trivial amount of work though.
> > > > >
> > > >
> > > > I have the luxory of not knowing the broker internals, so I can only
> > > > discuss
> > > > this on a conceptual design level.
> > > >
> > > > In its simplest form each API request type has a NeedsAuth flag and
> the
> > > > broker protocol request layer simply checks if the current session is
> > > > Authenticated
> > > > before processing the request: If not the session is closed and an
> > error
> > > is
> > > > logged.
> > > > The only two API requests that dont have the NeedsAuth flag would be
> > > > SaslHandshakeRequest
> > > > and ApiVersionRequest, the latter could also use filtering to only
> > return
> > > > the same two
> > > > requests in ApiVersionResponse before the client is authenticated (as
> > not
> > > > to "leak" information).
> > > > If authentication is not configured on the broker all sessions are
> > deemed
> > > > authenticated by default.
> > > >
> > > >
> > > > Re backwards compatibility:
> > > > My suggestion is to keep the current special SASL handshake protocol
> on
> > > the
> > > > SASL_PLAIN/SASL_SSL
> > > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API on
> > the
> > > > PLAIN/SSL endpoints.
> > > > This way the broker is backwards compatible with older clients that
> > only
> > > > supports the special SASL protocol,
> > > > and newer cliets are also backwards compatible with older brokers
> that
> > > only
> > > > supports the special SASL protocol.
> > > > Newer clients connecting to new brokers will be configured to use
> > > non-SASL
> > > > ports and use the
> > > > in-band Kafka SaslHandshakeRequest to authenticate.
> > > >
> > > > Using the existing standard Kafka protocol and the new future-proof
> > > > functionality of ApiVersionRequest
> > > > allows the in-band authentication mechanisms and semantics to
> naturally
> > > > evolve over time
> > > > without breaking existing clients.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > We started KIP-35 with supporting a client to know if a version is
> > > > > supported by the broker. It's now evolved into supporting a client
> to
> > > > > implement multiple versions of the protocol and dynamically pick a
> > > > version
> > > > > supported by the broker. The former is likely solvable without
> > > > > ApiVersionRequest. How important is the latter? What if the C
> client
> > > just
> > > > > follows the java client model by implementing one version of
> protocol
> > > > per C
> > > > > client release (which seems easier to implement)?
> > > > >
> > > >
> > > > We've discussed this at length and it is not an option for
> librdkafka,
> > > nor
> > > > kafka-python, and
> > > > probably other clients as well, due to usability/UX and maintenance
> > > > reasons.
> > > > (There's even discussion of making the Java client more version
> > > agnostic!)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > >
> > > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> > > > >
> > > > > > Magnus,
> > > > > >
> > > > > > A while back, we had another proposal for the broker to just send
> > the
> > > > > > correlation id and an empty payload if it receives an unsupported
> > > > version
> > > > > > of the request. I didn't see that in the rejected section. It
> seems
> > > > > simpler
> > > > > > than the current proposal where the client has to issue an
> > > > > > ApiVersionRequest on every connection. Could you document the
> > reason
> > > > why
> > > > > we
> > > > > > rejected it?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <
> asingh@cloudera.com>
> > > > > wrote:
> > > > > >
> > > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk>
> > > > wrote:
> > > > > >>
> > > > > >> > Two more things:
> > > > > >> >
> > > > > >> > 3. We talk about backporting of new request versions to stable
> > > > > branches
> > > > > >> in
> > > > > >> > the KIP. In practice, we can't do that until the Java client
> is
> > > > > changed
> > > > > >> so
> > > > > >> > that it doesn't blindly use the latest protocol version.
> > > Otherwise,
> > > > if
> > > > > >> new
> > > > > >> > request versions were added to 0.9.0.2, the client would break
> > > when
> > > > > >> talking
> > > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a
> bit
> > > > more
> > > > > >> > gracefully, but that's still not good enough for a stable
> > branch).
> > > > It
> > > > > >> may
> > > > > >> > be worth making this clear in the KIP (yes, it is a bit
> > orthogonal
> > > > and
> > > > > >> > doesn't prevent the KIP from being adopted, but good to avoid
> > > > > >> confusion).
> > > > > >> >
> > > > > >> Good point. Adding this note and also adding a note that Kafka
> has
> > > not
> > > > > >> backported an api version so far.
> > > > > >>
> > > > > >> >
> > > > > >> > 4. The paragraph below is a bit confusing. It starts talking
> > about
> > > > > 0.9.0
> > > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > > >> >
> > > > > >> Yes.
> > > > > >>
> > > > > >> >
> > > > > >> > "Deprecation of a protocol version will be done by marking a
> > > > protocol
> > > > > >> > version as deprecated in protocol documentation. Documentation
> > > shall
> > > > > >> also
> > > > > >> > be used to indicate a protocol version that must not be used,
> or
> > > for
> > > > > any
> > > > > >> > such information.For instance, say 0.9.0 had protocol versions
> > [0]
> > > > for
> > > > > >> api
> > > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> > running
> > > > off
> > > > > >> > trunk started using version 1 of the api and found out a major
> > > bug.
> > > > To
> > > > > >> > rectify that version 2 of the api is added to trunk. For some
> > > > reason,
> > > > > >> it is
> > > > > >> > now deemed important to have version 2 of the api in 0.9.1 as
> > > well.
> > > > To
> > > > > >> do
> > > > > >> > so, version 1 and version 2 both of the api will be backported
> > to
> > > > the
> > > > > >> 0.9.1
> > > > > >> > branch. 0.9.1 broker will return 0 as min supported version
> for
> > > the
> > > > > api
> > > > > >> and
> > > > > >> > 2 for the max supported version for the api. However, the
> > version
> > > 1
> > > > > >> should
> > > > > >> > be clearly marked as deprecated on its documentation. It will
> be
> > > > > >> client's
> > > > > >> > responsibility to make sure they are not using any such
> > deprecated
> > > > > >> version
> > > > > >> > to the best knowledge of the client at the time of development
> > (or
> > > > > >> > alternatively by configuration)."
> > > > > >> >
> > > > > >> > Ismael
> > > > > >> >
> > > > > >> >
> > > > > >> >
> > > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > >> >
> > > > > >> > > A couple of questions:
> > > > > >> > >
> > > > > >> > > 1. The KIP says "Specific version may be deprecated through
> > > > protocol
> > > > > >> > > documentation but must still be supported (although it is
> fair
> > > to
> > > > > >> return
> > > > > >> > an
> > > > > >> > > error code if the specific API supports it).". It may be
> worth
> > > > > >> expanding
> > > > > >> > > this a little more. For example, what does it mean to
> support
> > > the
> > > > > >> API? I
> > > > > >> > > guess this means that the broker must not disconnect the
> > client
> > > > and
> > > > > >> the
> > > > > >> > > broker must return a valid protocol response. Given that it
> > says
> > > > > that
> > > > > >> it
> > > > > >> > is
> > > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> > return
> > > an
> > > > > >> error
> > > > > >> > > code if the specific API supports it, it sounds like we are
> > > saying
> > > > > >> that
> > > > > >> > we
> > > > > >> > > don't have to maintain the semantic behaviour (i.e. we could
> > > > > _always_
> > > > > >> > > return an error for a deprecated API?). Is this true?
> > > > > >> > >
> > > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > > >> ApiVersionRequest?
> > > > > >> > >
> > > > > >> > > Ismael
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> --
> > > > > >>
> > > > > >> Regards,
> > > > > >> Ashish
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
Regards,
Rajini
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Jun Rao <ju...@confluent.io>.
Magnus,
That sounds reasonable. To reduce the changes on the server side, I'd
suggest the following minor tweaks on the proposal.
1. Continue supporting the separate SASL and SASL_SSL port.
On SASL port, we support the new sequence
ApiVersionRequest (optional), SaslHandshakeRequest, SASL tokens, regular
requests
On SASL_SSL port, we support the new sequence
SSL handshake bytes, ApiVersionRequest (optional), SaslHandshakeRequest,
SASL tokens, regular requests
2. We don't wrap SASL tokens in Kafka protocol. Similar to your argument
about SSL handshake, those SASL tokens are generated by SASL library and
Kafka doesn't really control its versioning. Kafka only controls the
selection of SASL mechanism, which will be versioned in
SaslHandshakeRequest.
Does that work for you?
Thanks,
Jun
On Mon, Apr 11, 2016 at 11:15 AM, Magnus Edenhill <ma...@edenhill.se>
wrote:
> Hey Jun, see inline
>
> 2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
>
> > Hi, Magnus,
> >
> > Let me understand your proposal in more details just from the client's
> > perspective. My understanding of your proposal is the following.
> >
> > On plaintext port, the client will send the following bytes in order.
> > ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> > enabled), regular requests
> >
> > On SSL port, the client will send the following bytes in order.
> > SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest, SASL
> > tokens (if SASL is enabled), regular requests
> >
>
>
> Yup!
> "SASL tokens" is a series of proper Kafka protocol SaslHandshakeRequests
> until
> the handshake is done.
>
>
>
> >
> > Is that right? Since we can use either SSL or SASL for authentication,
> it's
> > weird that in one case, we require ApiVersionRequest to happen before
> > authentication and in another case we require the reverse.
> >
>
> Since the SSL/TLS is standardised and taken care of for us by the SSL
> libraries it
> doesnt make sense to reimplement that on top of Kafka, so it isn't really
> comparable.
> But for SASL there is no standardised handshake protocol so we must either
> conceive one from scratch, or use the protocol that we already have
> (Kafka).
> For the initial SASL implementation in 0.9 the first option was chosen and
> while
> it required a new protocol implementation in supporting clients and the
> broker
> it served its purpose. But not for long, it already needs to evolve,
> and this gives us a golden[1] opportunity to make the implementation
> reusable, evolvable, less complex
> and in line with all our other protocol work, by using the protocol stack
> of Kafka which the
> broker and all clients already have in place.
>
> Not taking this chance and instead diverging the custom SASL handshake
> protocol
> even further from Kafka seems to me a weird choice.
>
> The current KIP-43 proposal does not have a clear compatibility story; it
> doesnt seem to be possible
> to upgrade clients before brokers, while this might be okay for the Java
> client, the KIP-35 discussion
> has hopefully proven that this assumption can't be made for the entire
> eco-system.
>
> Let me be clear that there isn't anything technically wrong with the KIP-43
> proposal (well,
> except for the hack to check byte[0] for 0x60 perhaps), but I'm worried the
> proposal will eventually lead to
> reimplementing Api Versioning, KIP-35, etc, in the custom SASL handshake,
> and this is just redundant,
> there is no technical reason for doing so and it'll just make protocol
> semantics and implementations more complex.
>
>
> Regards,
> Magnus
>
> [1]: Timing is good for this change since only two clients, Java and C,
> currently supports
> the existing SASL handshake so far.
>
>
> >
> > Thanks,
> >
> > Jun
> >
> > On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <ma...@edenhill.se>
> > wrote:
> >
> > > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> > >
> > > > Thinking about ApiVersionRequest a bit more. There are quite a few
> > things
> > > > special about it. In the ideal case, (1) its version should never
> > change;
> > > >
> > >
> > > The only thing we know of the future is that we dont know anything, we
> > > can't
> > > think of every possible future use case, that's why need to be able to
> > > evolve interfaces
> > > as requirements and use-cases change. This is the gist of KIP-35, and
> > > hampering
> > > KIP-35 itself, by not letting it also evolve, does not seem right to me
> > at
> > > all.
> > >
> > >
> > >
> > >
> > > > (2) it needs to be done before authentication (either SSL/SASL); (3)
> it
> > > is
> > > > required to be issued at the beginning of each connection but never
> > needs
> > > > to be issued again on the same connection. So, instead of modeling it
> > as
> > > a
> > > > regular request, it seems a cleaner approach is to just bake that
> into
> > > the
> > > > initial connection handshake even before the authentication layer. So
> > the
> > > > sequencing in a connection will be api discovery, authentication,
> > > followed
> > > > by regular requests. I am not sure we can still add this in a
> backward
> > > > compatible way now (e.g., not sure what the initial bytes from an SSL
> > > > connection will look like). Even if we can do this in a backward
> > > compatible
> > > > way, it's probably non-trivial amount of work though.
> > > >
> > >
> > > I have the luxory of not knowing the broker internals, so I can only
> > > discuss
> > > this on a conceptual design level.
> > >
> > > In its simplest form each API request type has a NeedsAuth flag and the
> > > broker protocol request layer simply checks if the current session is
> > > Authenticated
> > > before processing the request: If not the session is closed and an
> error
> > is
> > > logged.
> > > The only two API requests that dont have the NeedsAuth flag would be
> > > SaslHandshakeRequest
> > > and ApiVersionRequest, the latter could also use filtering to only
> return
> > > the same two
> > > requests in ApiVersionResponse before the client is authenticated (as
> not
> > > to "leak" information).
> > > If authentication is not configured on the broker all sessions are
> deemed
> > > authenticated by default.
> > >
> > >
> > > Re backwards compatibility:
> > > My suggestion is to keep the current special SASL handshake protocol on
> > the
> > > SASL_PLAIN/SASL_SSL
> > > endpoints, but use the new in-band Kafka SaslHandshakeRequest API on
> the
> > > PLAIN/SSL endpoints.
> > > This way the broker is backwards compatible with older clients that
> only
> > > supports the special SASL protocol,
> > > and newer cliets are also backwards compatible with older brokers that
> > only
> > > supports the special SASL protocol.
> > > Newer clients connecting to new brokers will be configured to use
> > non-SASL
> > > ports and use the
> > > in-band Kafka SaslHandshakeRequest to authenticate.
> > >
> > > Using the existing standard Kafka protocol and the new future-proof
> > > functionality of ApiVersionRequest
> > > allows the in-band authentication mechanisms and semantics to naturally
> > > evolve over time
> > > without breaking existing clients.
> > >
> > >
> > >
> > >
> > >
> > > >
> > > > We started KIP-35 with supporting a client to know if a version is
> > > > supported by the broker. It's now evolved into supporting a client to
> > > > implement multiple versions of the protocol and dynamically pick a
> > > version
> > > > supported by the broker. The former is likely solvable without
> > > > ApiVersionRequest. How important is the latter? What if the C client
> > just
> > > > follows the java client model by implementing one version of protocol
> > > per C
> > > > client release (which seems easier to implement)?
> > > >
> > >
> > > We've discussed this at length and it is not an option for librdkafka,
> > nor
> > > kafka-python, and
> > > probably other clients as well, due to usability/UX and maintenance
> > > reasons.
> > > (There's even discussion of making the Java client more version
> > agnostic!)
> > >
> > >
> > >
> > >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > >
> > > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> > > >
> > > > > Magnus,
> > > > >
> > > > > A while back, we had another proposal for the broker to just send
> the
> > > > > correlation id and an empty payload if it receives an unsupported
> > > version
> > > > > of the request. I didn't see that in the rejected section. It seems
> > > > simpler
> > > > > than the current proposal where the client has to issue an
> > > > > ApiVersionRequest on every connection. Could you document the
> reason
> > > why
> > > > we
> > > > > rejected it?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com>
> > > > wrote:
> > > > >
> > > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk>
> > > wrote:
> > > > >>
> > > > >> > Two more things:
> > > > >> >
> > > > >> > 3. We talk about backporting of new request versions to stable
> > > > branches
> > > > >> in
> > > > >> > the KIP. In practice, we can't do that until the Java client is
> > > > changed
> > > > >> so
> > > > >> > that it doesn't blindly use the latest protocol version.
> > Otherwise,
> > > if
> > > > >> new
> > > > >> > request versions were added to 0.9.0.2, the client would break
> > when
> > > > >> talking
> > > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit
> > > more
> > > > >> > gracefully, but that's still not good enough for a stable
> branch).
> > > It
> > > > >> may
> > > > >> > be worth making this clear in the KIP (yes, it is a bit
> orthogonal
> > > and
> > > > >> > doesn't prevent the KIP from being adopted, but good to avoid
> > > > >> confusion).
> > > > >> >
> > > > >> Good point. Adding this note and also adding a note that Kafka has
> > not
> > > > >> backported an api version so far.
> > > > >>
> > > > >> >
> > > > >> > 4. The paragraph below is a bit confusing. It starts talking
> about
> > > > 0.9.0
> > > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > > >> >
> > > > >> Yes.
> > > > >>
> > > > >> >
> > > > >> > "Deprecation of a protocol version will be done by marking a
> > > protocol
> > > > >> > version as deprecated in protocol documentation. Documentation
> > shall
> > > > >> also
> > > > >> > be used to indicate a protocol version that must not be used, or
> > for
> > > > any
> > > > >> > such information.For instance, say 0.9.0 had protocol versions
> [0]
> > > for
> > > > >> api
> > > > >> > key 1. On trunk, version 1 of the api key was added. Users
> running
> > > off
> > > > >> > trunk started using version 1 of the api and found out a major
> > bug.
> > > To
> > > > >> > rectify that version 2 of the api is added to trunk. For some
> > > reason,
> > > > >> it is
> > > > >> > now deemed important to have version 2 of the api in 0.9.1 as
> > well.
> > > To
> > > > >> do
> > > > >> > so, version 1 and version 2 both of the api will be backported
> to
> > > the
> > > > >> 0.9.1
> > > > >> > branch. 0.9.1 broker will return 0 as min supported version for
> > the
> > > > api
> > > > >> and
> > > > >> > 2 for the max supported version for the api. However, the
> version
> > 1
> > > > >> should
> > > > >> > be clearly marked as deprecated on its documentation. It will be
> > > > >> client's
> > > > >> > responsibility to make sure they are not using any such
> deprecated
> > > > >> version
> > > > >> > to the best knowledge of the client at the time of development
> (or
> > > > >> > alternatively by configuration)."
> > > > >> >
> > > > >> > Ismael
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk>
> > > > wrote:
> > > > >> >
> > > > >> > > A couple of questions:
> > > > >> > >
> > > > >> > > 1. The KIP says "Specific version may be deprecated through
> > > protocol
> > > > >> > > documentation but must still be supported (although it is fair
> > to
> > > > >> return
> > > > >> > an
> > > > >> > > error code if the specific API supports it).". It may be worth
> > > > >> expanding
> > > > >> > > this a little more. For example, what does it mean to support
> > the
> > > > >> API? I
> > > > >> > > guess this means that the broker must not disconnect the
> client
> > > and
> > > > >> the
> > > > >> > > broker must return a valid protocol response. Given that it
> says
> > > > that
> > > > >> it
> > > > >> > is
> > > > >> > > "fair" (I would probably replace "fair" with "valid") to
> return
> > an
> > > > >> error
> > > > >> > > code if the specific API supports it, it sounds like we are
> > saying
> > > > >> that
> > > > >> > we
> > > > >> > > don't have to maintain the semantic behaviour (i.e. we could
> > > > _always_
> > > > >> > > return an error for a deprecated API?). Is this true?
> > > > >> > >
> > > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > > >> ApiVersionRequest?
> > > > >> > >
> > > > >> > > Ismael
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >>
> > > > >> Regards,
> > > > >> Ashish
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Magnus Edenhill <ma...@edenhill.se>.
Hey Jun, see inline
2016-04-11 19:19 GMT+02:00 Jun Rao <ju...@confluent.io>:
> Hi, Magnus,
>
> Let me understand your proposal in more details just from the client's
> perspective. My understanding of your proposal is the following.
>
> On plaintext port, the client will send the following bytes in order.
> ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
> enabled), regular requests
>
> On SSL port, the client will send the following bytes in order.
> SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest, SASL
> tokens (if SASL is enabled), regular requests
>
Yup!
"SASL tokens" is a series of proper Kafka protocol SaslHandshakeRequests
until
the handshake is done.
>
> Is that right? Since we can use either SSL or SASL for authentication, it's
> weird that in one case, we require ApiVersionRequest to happen before
> authentication and in another case we require the reverse.
>
Since the SSL/TLS is standardised and taken care of for us by the SSL
libraries it
doesnt make sense to reimplement that on top of Kafka, so it isn't really
comparable.
But for SASL there is no standardised handshake protocol so we must either
conceive one from scratch, or use the protocol that we already have (Kafka).
For the initial SASL implementation in 0.9 the first option was chosen and
while
it required a new protocol implementation in supporting clients and the
broker
it served its purpose. But not for long, it already needs to evolve,
and this gives us a golden[1] opportunity to make the implementation
reusable, evolvable, less complex
and in line with all our other protocol work, by using the protocol stack
of Kafka which the
broker and all clients already have in place.
Not taking this chance and instead diverging the custom SASL handshake
protocol
even further from Kafka seems to me a weird choice.
The current KIP-43 proposal does not have a clear compatibility story; it
doesnt seem to be possible
to upgrade clients before brokers, while this might be okay for the Java
client, the KIP-35 discussion
has hopefully proven that this assumption can't be made for the entire
eco-system.
Let me be clear that there isn't anything technically wrong with the KIP-43
proposal (well,
except for the hack to check byte[0] for 0x60 perhaps), but I'm worried the
proposal will eventually lead to
reimplementing Api Versioning, KIP-35, etc, in the custom SASL handshake,
and this is just redundant,
there is no technical reason for doing so and it'll just make protocol
semantics and implementations more complex.
Regards,
Magnus
[1]: Timing is good for this change since only two clients, Java and C,
currently supports
the existing SASL handshake so far.
>
> Thanks,
>
> Jun
>
> On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <ma...@edenhill.se>
> wrote:
>
> > 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> >
> > > Thinking about ApiVersionRequest a bit more. There are quite a few
> things
> > > special about it. In the ideal case, (1) its version should never
> change;
> > >
> >
> > The only thing we know of the future is that we dont know anything, we
> > can't
> > think of every possible future use case, that's why need to be able to
> > evolve interfaces
> > as requirements and use-cases change. This is the gist of KIP-35, and
> > hampering
> > KIP-35 itself, by not letting it also evolve, does not seem right to me
> at
> > all.
> >
> >
> >
> >
> > > (2) it needs to be done before authentication (either SSL/SASL); (3) it
> > is
> > > required to be issued at the beginning of each connection but never
> needs
> > > to be issued again on the same connection. So, instead of modeling it
> as
> > a
> > > regular request, it seems a cleaner approach is to just bake that into
> > the
> > > initial connection handshake even before the authentication layer. So
> the
> > > sequencing in a connection will be api discovery, authentication,
> > followed
> > > by regular requests. I am not sure we can still add this in a backward
> > > compatible way now (e.g., not sure what the initial bytes from an SSL
> > > connection will look like). Even if we can do this in a backward
> > compatible
> > > way, it's probably non-trivial amount of work though.
> > >
> >
> > I have the luxory of not knowing the broker internals, so I can only
> > discuss
> > this on a conceptual design level.
> >
> > In its simplest form each API request type has a NeedsAuth flag and the
> > broker protocol request layer simply checks if the current session is
> > Authenticated
> > before processing the request: If not the session is closed and an error
> is
> > logged.
> > The only two API requests that dont have the NeedsAuth flag would be
> > SaslHandshakeRequest
> > and ApiVersionRequest, the latter could also use filtering to only return
> > the same two
> > requests in ApiVersionResponse before the client is authenticated (as not
> > to "leak" information).
> > If authentication is not configured on the broker all sessions are deemed
> > authenticated by default.
> >
> >
> > Re backwards compatibility:
> > My suggestion is to keep the current special SASL handshake protocol on
> the
> > SASL_PLAIN/SASL_SSL
> > endpoints, but use the new in-band Kafka SaslHandshakeRequest API on the
> > PLAIN/SSL endpoints.
> > This way the broker is backwards compatible with older clients that only
> > supports the special SASL protocol,
> > and newer cliets are also backwards compatible with older brokers that
> only
> > supports the special SASL protocol.
> > Newer clients connecting to new brokers will be configured to use
> non-SASL
> > ports and use the
> > in-band Kafka SaslHandshakeRequest to authenticate.
> >
> > Using the existing standard Kafka protocol and the new future-proof
> > functionality of ApiVersionRequest
> > allows the in-band authentication mechanisms and semantics to naturally
> > evolve over time
> > without breaking existing clients.
> >
> >
> >
> >
> >
> > >
> > > We started KIP-35 with supporting a client to know if a version is
> > > supported by the broker. It's now evolved into supporting a client to
> > > implement multiple versions of the protocol and dynamically pick a
> > version
> > > supported by the broker. The former is likely solvable without
> > > ApiVersionRequest. How important is the latter? What if the C client
> just
> > > follows the java client model by implementing one version of protocol
> > per C
> > > client release (which seems easier to implement)?
> > >
> >
> > We've discussed this at length and it is not an option for librdkafka,
> nor
> > kafka-python, and
> > probably other clients as well, due to usability/UX and maintenance
> > reasons.
> > (There's even discussion of making the Java client more version
> agnostic!)
> >
> >
> >
> >
> >
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > >
> > > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> > >
> > > > Magnus,
> > > >
> > > > A while back, we had another proposal for the broker to just send the
> > > > correlation id and an empty payload if it receives an unsupported
> > version
> > > > of the request. I didn't see that in the rejected section. It seems
> > > simpler
> > > > than the current proposal where the client has to issue an
> > > > ApiVersionRequest on every connection. Could you document the reason
> > why
> > > we
> > > > rejected it?
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com>
> > > wrote:
> > > >
> > > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > >>
> > > >> > Two more things:
> > > >> >
> > > >> > 3. We talk about backporting of new request versions to stable
> > > branches
> > > >> in
> > > >> > the KIP. In practice, we can't do that until the Java client is
> > > changed
> > > >> so
> > > >> > that it doesn't blindly use the latest protocol version.
> Otherwise,
> > if
> > > >> new
> > > >> > request versions were added to 0.9.0.2, the client would break
> when
> > > >> talking
> > > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit
> > more
> > > >> > gracefully, but that's still not good enough for a stable branch).
> > It
> > > >> may
> > > >> > be worth making this clear in the KIP (yes, it is a bit orthogonal
> > and
> > > >> > doesn't prevent the KIP from being adopted, but good to avoid
> > > >> confusion).
> > > >> >
> > > >> Good point. Adding this note and also adding a note that Kafka has
> not
> > > >> backported an api version so far.
> > > >>
> > > >> >
> > > >> > 4. The paragraph below is a bit confusing. It starts talking about
> > > 0.9.0
> > > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > > >> >
> > > >> Yes.
> > > >>
> > > >> >
> > > >> > "Deprecation of a protocol version will be done by marking a
> > protocol
> > > >> > version as deprecated in protocol documentation. Documentation
> shall
> > > >> also
> > > >> > be used to indicate a protocol version that must not be used, or
> for
> > > any
> > > >> > such information.For instance, say 0.9.0 had protocol versions [0]
> > for
> > > >> api
> > > >> > key 1. On trunk, version 1 of the api key was added. Users running
> > off
> > > >> > trunk started using version 1 of the api and found out a major
> bug.
> > To
> > > >> > rectify that version 2 of the api is added to trunk. For some
> > reason,
> > > >> it is
> > > >> > now deemed important to have version 2 of the api in 0.9.1 as
> well.
> > To
> > > >> do
> > > >> > so, version 1 and version 2 both of the api will be backported to
> > the
> > > >> 0.9.1
> > > >> > branch. 0.9.1 broker will return 0 as min supported version for
> the
> > > api
> > > >> and
> > > >> > 2 for the max supported version for the api. However, the version
> 1
> > > >> should
> > > >> > be clearly marked as deprecated on its documentation. It will be
> > > >> client's
> > > >> > responsibility to make sure they are not using any such deprecated
> > > >> version
> > > >> > to the best knowledge of the client at the time of development (or
> > > >> > alternatively by configuration)."
> > > >> >
> > > >> > Ismael
> > > >> >
> > > >> >
> > > >> >
> > > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk>
> > > wrote:
> > > >> >
> > > >> > > A couple of questions:
> > > >> > >
> > > >> > > 1. The KIP says "Specific version may be deprecated through
> > protocol
> > > >> > > documentation but must still be supported (although it is fair
> to
> > > >> return
> > > >> > an
> > > >> > > error code if the specific API supports it).". It may be worth
> > > >> expanding
> > > >> > > this a little more. For example, what does it mean to support
> the
> > > >> API? I
> > > >> > > guess this means that the broker must not disconnect the client
> > and
> > > >> the
> > > >> > > broker must return a valid protocol response. Given that it says
> > > that
> > > >> it
> > > >> > is
> > > >> > > "fair" (I would probably replace "fair" with "valid") to return
> an
> > > >> error
> > > >> > > code if the specific API supports it, it sounds like we are
> saying
> > > >> that
> > > >> > we
> > > >> > > don't have to maintain the semantic behaviour (i.e. we could
> > > _always_
> > > >> > > return an error for a deprecated API?). Is this true?
> > > >> > >
> > > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > > >> ApiVersionRequest?
> > > >> > >
> > > >> > > Ismael
> > > >> > >
> > > >> >
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >>
> > > >> Regards,
> > > >> Ashish
> > > >>
> > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Jun Rao <ju...@confluent.io>.
Hi, Magnus,
Let me understand your proposal in more details just from the client's
perspective. My understanding of your proposal is the following.
On plaintext port, the client will send the following bytes in order.
ApiVersionRequest, SaslHandshakeRequest, SASL tokens (if SASL is
enabled), regular requests
On SSL port, the client will send the following bytes in order.
SSL handshake bytes, ApiVersionRequest, SaslHandshakeRequest, SASL
tokens (if SASL is enabled), regular requests
Is that right? Since we can use either SSL or SASL for authentication, it's
weird that in one case, we require ApiVersionRequest to happen before
authentication and in another case we require the reverse.
Thanks,
Jun
On Mon, Apr 11, 2016 at 12:20 AM, Magnus Edenhill <ma...@edenhill.se>
wrote:
> 2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
>
> > Thinking about ApiVersionRequest a bit more. There are quite a few things
> > special about it. In the ideal case, (1) its version should never change;
> >
>
> The only thing we know of the future is that we dont know anything, we
> can't
> think of every possible future use case, that's why need to be able to
> evolve interfaces
> as requirements and use-cases change. This is the gist of KIP-35, and
> hampering
> KIP-35 itself, by not letting it also evolve, does not seem right to me at
> all.
>
>
>
>
> > (2) it needs to be done before authentication (either SSL/SASL); (3) it
> is
> > required to be issued at the beginning of each connection but never needs
> > to be issued again on the same connection. So, instead of modeling it as
> a
> > regular request, it seems a cleaner approach is to just bake that into
> the
> > initial connection handshake even before the authentication layer. So the
> > sequencing in a connection will be api discovery, authentication,
> followed
> > by regular requests. I am not sure we can still add this in a backward
> > compatible way now (e.g., not sure what the initial bytes from an SSL
> > connection will look like). Even if we can do this in a backward
> compatible
> > way, it's probably non-trivial amount of work though.
> >
>
> I have the luxory of not knowing the broker internals, so I can only
> discuss
> this on a conceptual design level.
>
> In its simplest form each API request type has a NeedsAuth flag and the
> broker protocol request layer simply checks if the current session is
> Authenticated
> before processing the request: If not the session is closed and an error is
> logged.
> The only two API requests that dont have the NeedsAuth flag would be
> SaslHandshakeRequest
> and ApiVersionRequest, the latter could also use filtering to only return
> the same two
> requests in ApiVersionResponse before the client is authenticated (as not
> to "leak" information).
> If authentication is not configured on the broker all sessions are deemed
> authenticated by default.
>
>
> Re backwards compatibility:
> My suggestion is to keep the current special SASL handshake protocol on the
> SASL_PLAIN/SASL_SSL
> endpoints, but use the new in-band Kafka SaslHandshakeRequest API on the
> PLAIN/SSL endpoints.
> This way the broker is backwards compatible with older clients that only
> supports the special SASL protocol,
> and newer cliets are also backwards compatible with older brokers that only
> supports the special SASL protocol.
> Newer clients connecting to new brokers will be configured to use non-SASL
> ports and use the
> in-band Kafka SaslHandshakeRequest to authenticate.
>
> Using the existing standard Kafka protocol and the new future-proof
> functionality of ApiVersionRequest
> allows the in-band authentication mechanisms and semantics to naturally
> evolve over time
> without breaking existing clients.
>
>
>
>
>
> >
> > We started KIP-35 with supporting a client to know if a version is
> > supported by the broker. It's now evolved into supporting a client to
> > implement multiple versions of the protocol and dynamically pick a
> version
> > supported by the broker. The former is likely solvable without
> > ApiVersionRequest. How important is the latter? What if the C client just
> > follows the java client model by implementing one version of protocol
> per C
> > client release (which seems easier to implement)?
> >
>
> We've discussed this at length and it is not an option for librdkafka, nor
> kafka-python, and
> probably other clients as well, due to usability/UX and maintenance
> reasons.
> (There's even discussion of making the Java client more version agnostic!)
>
>
>
>
>
> >
> > Thanks,
> >
> > Jun
> >
> >
> > On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Magnus,
> > >
> > > A while back, we had another proposal for the broker to just send the
> > > correlation id and an empty payload if it receives an unsupported
> version
> > > of the request. I didn't see that in the rejected section. It seems
> > simpler
> > > than the current proposal where the client has to issue an
> > > ApiVersionRequest on every connection. Could you document the reason
> why
> > we
> > > rejected it?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com>
> > wrote:
> > >
> > >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk>
> wrote:
> > >>
> > >> > Two more things:
> > >> >
> > >> > 3. We talk about backporting of new request versions to stable
> > branches
> > >> in
> > >> > the KIP. In practice, we can't do that until the Java client is
> > changed
> > >> so
> > >> > that it doesn't blindly use the latest protocol version. Otherwise,
> if
> > >> new
> > >> > request versions were added to 0.9.0.2, the client would break when
> > >> talking
> > >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit
> more
> > >> > gracefully, but that's still not good enough for a stable branch).
> It
> > >> may
> > >> > be worth making this clear in the KIP (yes, it is a bit orthogonal
> and
> > >> > doesn't prevent the KIP from being adopted, but good to avoid
> > >> confusion).
> > >> >
> > >> Good point. Adding this note and also adding a note that Kafka has not
> > >> backported an api version so far.
> > >>
> > >> >
> > >> > 4. The paragraph below is a bit confusing. It starts talking about
> > 0.9.0
> > >> > and trunk and then switches to 0.9.1. Is that intentional?
> > >> >
> > >> Yes.
> > >>
> > >> >
> > >> > "Deprecation of a protocol version will be done by marking a
> protocol
> > >> > version as deprecated in protocol documentation. Documentation shall
> > >> also
> > >> > be used to indicate a protocol version that must not be used, or for
> > any
> > >> > such information.For instance, say 0.9.0 had protocol versions [0]
> for
> > >> api
> > >> > key 1. On trunk, version 1 of the api key was added. Users running
> off
> > >> > trunk started using version 1 of the api and found out a major bug.
> To
> > >> > rectify that version 2 of the api is added to trunk. For some
> reason,
> > >> it is
> > >> > now deemed important to have version 2 of the api in 0.9.1 as well.
> To
> > >> do
> > >> > so, version 1 and version 2 both of the api will be backported to
> the
> > >> 0.9.1
> > >> > branch. 0.9.1 broker will return 0 as min supported version for the
> > api
> > >> and
> > >> > 2 for the max supported version for the api. However, the version 1
> > >> should
> > >> > be clearly marked as deprecated on its documentation. It will be
> > >> client's
> > >> > responsibility to make sure they are not using any such deprecated
> > >> version
> > >> > to the best knowledge of the client at the time of development (or
> > >> > alternatively by configuration)."
> > >> >
> > >> > Ismael
> > >> >
> > >> >
> > >> >
> > >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk>
> > wrote:
> > >> >
> > >> > > A couple of questions:
> > >> > >
> > >> > > 1. The KIP says "Specific version may be deprecated through
> protocol
> > >> > > documentation but must still be supported (although it is fair to
> > >> return
> > >> > an
> > >> > > error code if the specific API supports it).". It may be worth
> > >> expanding
> > >> > > this a little more. For example, what does it mean to support the
> > >> API? I
> > >> > > guess this means that the broker must not disconnect the client
> and
> > >> the
> > >> > > broker must return a valid protocol response. Given that it says
> > that
> > >> it
> > >> > is
> > >> > > "fair" (I would probably replace "fair" with "valid") to return an
> > >> error
> > >> > > code if the specific API supports it, it sounds like we are saying
> > >> that
> > >> > we
> > >> > > don't have to maintain the semantic behaviour (i.e. we could
> > _always_
> > >> > > return an error for a deprecated API?). Is this true?
> > >> > >
> > >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > >> ApiVersionRequest?
> > >> > >
> > >> > > Ismael
> > >> > >
> > >> >
> > >>
> > >>
> > >>
> > >> --
> > >>
> > >> Regards,
> > >> Ashish
> > >>
> > >
> > >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Magnus Edenhill <ma...@edenhill.se>.
2016-04-11 3:01 GMT+02:00 Jun Rao <ju...@confluent.io>:
> Thinking about ApiVersionRequest a bit more. There are quite a few things
> special about it. In the ideal case, (1) its version should never change;
>
The only thing we know of the future is that we dont know anything, we can't
think of every possible future use case, that's why need to be able to
evolve interfaces
as requirements and use-cases change. This is the gist of KIP-35, and
hampering
KIP-35 itself, by not letting it also evolve, does not seem right to me at
all.
> (2) it needs to be done before authentication (either SSL/SASL); (3) it is
> required to be issued at the beginning of each connection but never needs
> to be issued again on the same connection. So, instead of modeling it as a
> regular request, it seems a cleaner approach is to just bake that into the
> initial connection handshake even before the authentication layer. So the
> sequencing in a connection will be api discovery, authentication, followed
> by regular requests. I am not sure we can still add this in a backward
> compatible way now (e.g., not sure what the initial bytes from an SSL
> connection will look like). Even if we can do this in a backward compatible
> way, it's probably non-trivial amount of work though.
>
I have the luxory of not knowing the broker internals, so I can only discuss
this on a conceptual design level.
In its simplest form each API request type has a NeedsAuth flag and the
broker protocol request layer simply checks if the current session is
Authenticated
before processing the request: If not the session is closed and an error is
logged.
The only two API requests that dont have the NeedsAuth flag would be
SaslHandshakeRequest
and ApiVersionRequest, the latter could also use filtering to only return
the same two
requests in ApiVersionResponse before the client is authenticated (as not
to "leak" information).
If authentication is not configured on the broker all sessions are deemed
authenticated by default.
Re backwards compatibility:
My suggestion is to keep the current special SASL handshake protocol on the
SASL_PLAIN/SASL_SSL
endpoints, but use the new in-band Kafka SaslHandshakeRequest API on the
PLAIN/SSL endpoints.
This way the broker is backwards compatible with older clients that only
supports the special SASL protocol,
and newer cliets are also backwards compatible with older brokers that only
supports the special SASL protocol.
Newer clients connecting to new brokers will be configured to use non-SASL
ports and use the
in-band Kafka SaslHandshakeRequest to authenticate.
Using the existing standard Kafka protocol and the new future-proof
functionality of ApiVersionRequest
allows the in-band authentication mechanisms and semantics to naturally
evolve over time
without breaking existing clients.
>
> We started KIP-35 with supporting a client to know if a version is
> supported by the broker. It's now evolved into supporting a client to
> implement multiple versions of the protocol and dynamically pick a version
> supported by the broker. The former is likely solvable without
> ApiVersionRequest. How important is the latter? What if the C client just
> follows the java client model by implementing one version of protocol per C
> client release (which seems easier to implement)?
>
We've discussed this at length and it is not an option for librdkafka, nor
kafka-python, and
probably other clients as well, due to usability/UX and maintenance reasons.
(There's even discussion of making the Java client more version agnostic!)
>
> Thanks,
>
> Jun
>
>
> On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
>
> > Magnus,
> >
> > A while back, we had another proposal for the broker to just send the
> > correlation id and an empty payload if it receives an unsupported version
> > of the request. I didn't see that in the rejected section. It seems
> simpler
> > than the current proposal where the client has to issue an
> > ApiVersionRequest on every connection. Could you document the reason why
> we
> > rejected it?
> >
> > Thanks,
> >
> > Jun
> >
> > On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com>
> wrote:
> >
> >> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >>
> >> > Two more things:
> >> >
> >> > 3. We talk about backporting of new request versions to stable
> branches
> >> in
> >> > the KIP. In practice, we can't do that until the Java client is
> changed
> >> so
> >> > that it doesn't blindly use the latest protocol version. Otherwise, if
> >> new
> >> > request versions were added to 0.9.0.2, the client would break when
> >> talking
> >> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
> >> > gracefully, but that's still not good enough for a stable branch). It
> >> may
> >> > be worth making this clear in the KIP (yes, it is a bit orthogonal and
> >> > doesn't prevent the KIP from being adopted, but good to avoid
> >> confusion).
> >> >
> >> Good point. Adding this note and also adding a note that Kafka has not
> >> backported an api version so far.
> >>
> >> >
> >> > 4. The paragraph below is a bit confusing. It starts talking about
> 0.9.0
> >> > and trunk and then switches to 0.9.1. Is that intentional?
> >> >
> >> Yes.
> >>
> >> >
> >> > "Deprecation of a protocol version will be done by marking a protocol
> >> > version as deprecated in protocol documentation. Documentation shall
> >> also
> >> > be used to indicate a protocol version that must not be used, or for
> any
> >> > such information.For instance, say 0.9.0 had protocol versions [0] for
> >> api
> >> > key 1. On trunk, version 1 of the api key was added. Users running off
> >> > trunk started using version 1 of the api and found out a major bug. To
> >> > rectify that version 2 of the api is added to trunk. For some reason,
> >> it is
> >> > now deemed important to have version 2 of the api in 0.9.1 as well. To
> >> do
> >> > so, version 1 and version 2 both of the api will be backported to the
> >> 0.9.1
> >> > branch. 0.9.1 broker will return 0 as min supported version for the
> api
> >> and
> >> > 2 for the max supported version for the api. However, the version 1
> >> should
> >> > be clearly marked as deprecated on its documentation. It will be
> >> client's
> >> > responsibility to make sure they are not using any such deprecated
> >> version
> >> > to the best knowledge of the client at the time of development (or
> >> > alternatively by configuration)."
> >> >
> >> > Ismael
> >> >
> >> >
> >> >
> >> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk>
> wrote:
> >> >
> >> > > A couple of questions:
> >> > >
> >> > > 1. The KIP says "Specific version may be deprecated through protocol
> >> > > documentation but must still be supported (although it is fair to
> >> return
> >> > an
> >> > > error code if the specific API supports it).". It may be worth
> >> expanding
> >> > > this a little more. For example, what does it mean to support the
> >> API? I
> >> > > guess this means that the broker must not disconnect the client and
> >> the
> >> > > broker must return a valid protocol response. Given that it says
> that
> >> it
> >> > is
> >> > > "fair" (I would probably replace "fair" with "valid") to return an
> >> error
> >> > > code if the specific API supports it, it sounds like we are saying
> >> that
> >> > we
> >> > > don't have to maintain the semantic behaviour (i.e. we could
> _always_
> >> > > return an error for a deprecated API?). Is this true?
> >> > >
> >> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> >> ApiVersionRequest?
> >> > >
> >> > > Ismael
> >> > >
> >> >
> >>
> >>
> >>
> >> --
> >>
> >> Regards,
> >> Ashish
> >>
> >
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Jun Rao <ju...@confluent.io>.
Thinking about ApiVersionRequest a bit more. There are quite a few things
special about it. In the ideal case, (1) its version should never change;
(2) it needs to be done before authentication (either SSL/SASL); (3) it is
required to be issued at the beginning of each connection but never needs
to be issued again on the same connection. So, instead of modeling it as a
regular request, it seems a cleaner approach is to just bake that into the
initial connection handshake even before the authentication layer. So the
sequencing in a connection will be api discovery, authentication, followed
by regular requests. I am not sure we can still add this in a backward
compatible way now (e.g., not sure what the initial bytes from an SSL
connection will look like). Even if we can do this in a backward compatible
way, it's probably non-trivial amount of work though.
We started KIP-35 with supporting a client to know if a version is
supported by the broker. It's now evolved into supporting a client to
implement multiple versions of the protocol and dynamically pick a version
supported by the broker. The former is likely solvable without
ApiVersionRequest. How important is the latter? What if the C client just
follows the java client model by implementing one version of protocol per C
client release (which seems easier to implement)?
Thanks,
Jun
On Fri, Apr 8, 2016 at 4:20 PM, Jun Rao <ju...@confluent.io> wrote:
> Magnus,
>
> A while back, we had another proposal for the broker to just send the
> correlation id and an empty payload if it receives an unsupported version
> of the request. I didn't see that in the rejected section. It seems simpler
> than the current proposal where the client has to issue an
> ApiVersionRequest on every connection. Could you document the reason why we
> rejected it?
>
> Thanks,
>
> Jun
>
> On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com> wrote:
>
>> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk> wrote:
>>
>> > Two more things:
>> >
>> > 3. We talk about backporting of new request versions to stable branches
>> in
>> > the KIP. In practice, we can't do that until the Java client is changed
>> so
>> > that it doesn't blindly use the latest protocol version. Otherwise, if
>> new
>> > request versions were added to 0.9.0.2, the client would break when
>> talking
>> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
>> > gracefully, but that's still not good enough for a stable branch). It
>> may
>> > be worth making this clear in the KIP (yes, it is a bit orthogonal and
>> > doesn't prevent the KIP from being adopted, but good to avoid
>> confusion).
>> >
>> Good point. Adding this note and also adding a note that Kafka has not
>> backported an api version so far.
>>
>> >
>> > 4. The paragraph below is a bit confusing. It starts talking about 0.9.0
>> > and trunk and then switches to 0.9.1. Is that intentional?
>> >
>> Yes.
>>
>> >
>> > "Deprecation of a protocol version will be done by marking a protocol
>> > version as deprecated in protocol documentation. Documentation shall
>> also
>> > be used to indicate a protocol version that must not be used, or for any
>> > such information.For instance, say 0.9.0 had protocol versions [0] for
>> api
>> > key 1. On trunk, version 1 of the api key was added. Users running off
>> > trunk started using version 1 of the api and found out a major bug. To
>> > rectify that version 2 of the api is added to trunk. For some reason,
>> it is
>> > now deemed important to have version 2 of the api in 0.9.1 as well. To
>> do
>> > so, version 1 and version 2 both of the api will be backported to the
>> 0.9.1
>> > branch. 0.9.1 broker will return 0 as min supported version for the api
>> and
>> > 2 for the max supported version for the api. However, the version 1
>> should
>> > be clearly marked as deprecated on its documentation. It will be
>> client's
>> > responsibility to make sure they are not using any such deprecated
>> version
>> > to the best knowledge of the client at the time of development (or
>> > alternatively by configuration)."
>> >
>> > Ismael
>> >
>> >
>> >
>> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
>> >
>> > > A couple of questions:
>> > >
>> > > 1. The KIP says "Specific version may be deprecated through protocol
>> > > documentation but must still be supported (although it is fair to
>> return
>> > an
>> > > error code if the specific API supports it).". It may be worth
>> expanding
>> > > this a little more. For example, what does it mean to support the
>> API? I
>> > > guess this means that the broker must not disconnect the client and
>> the
>> > > broker must return a valid protocol response. Given that it says that
>> it
>> > is
>> > > "fair" (I would probably replace "fair" with "valid") to return an
>> error
>> > > code if the specific API supports it, it sounds like we are saying
>> that
>> > we
>> > > don't have to maintain the semantic behaviour (i.e. we could _always_
>> > > return an error for a deprecated API?). Is this true?
>> > >
>> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
>> ApiVersionRequest?
>> > >
>> > > Ismael
>> > >
>> >
>>
>>
>>
>> --
>>
>> Regards,
>> Ashish
>>
>
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Magnus Edenhill <ma...@edenhill.se>.
Hi Jun,
the "send empty response instead of disconnect on unsupported protocol"
was actually a sub-proposal and was never meant as an alternative to the
proper feature detection proposed by KIP-35.
I've added it back to the list of rejected alternatives on the wiki page,
thanks
for pointing this out. There's an example and motivation on there too.
/Magnus
2016-04-09 1:20 GMT+02:00 Jun Rao <ju...@confluent.io>:
> Magnus,
>
> A while back, we had another proposal for the broker to just send the
> correlation id and an empty payload if it receives an unsupported version
> of the request. I didn't see that in the rejected section. It seems simpler
> than the current proposal where the client has to issue an
> ApiVersionRequest on every connection. Could you document the reason why we
> rejected it?
>
> Thanks,
>
> Jun
>
> On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com> wrote:
>
> > On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Two more things:
> > >
> > > 3. We talk about backporting of new request versions to stable branches
> > in
> > > the KIP. In practice, we can't do that until the Java client is changed
> > so
> > > that it doesn't blindly use the latest protocol version. Otherwise, if
> > new
> > > request versions were added to 0.9.0.2, the client would break when
> > talking
> > > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
> > > gracefully, but that's still not good enough for a stable branch). It
> may
> > > be worth making this clear in the KIP (yes, it is a bit orthogonal and
> > > doesn't prevent the KIP from being adopted, but good to avoid
> confusion).
> > >
> > Good point. Adding this note and also adding a note that Kafka has not
> > backported an api version so far.
> >
> > >
> > > 4. The paragraph below is a bit confusing. It starts talking about
> 0.9.0
> > > and trunk and then switches to 0.9.1. Is that intentional?
> > >
> > Yes.
> >
> > >
> > > "Deprecation of a protocol version will be done by marking a protocol
> > > version as deprecated in protocol documentation. Documentation shall
> also
> > > be used to indicate a protocol version that must not be used, or for
> any
> > > such information.For instance, say 0.9.0 had protocol versions [0] for
> > api
> > > key 1. On trunk, version 1 of the api key was added. Users running off
> > > trunk started using version 1 of the api and found out a major bug. To
> > > rectify that version 2 of the api is added to trunk. For some reason,
> it
> > is
> > > now deemed important to have version 2 of the api in 0.9.1 as well. To
> do
> > > so, version 1 and version 2 both of the api will be backported to the
> > 0.9.1
> > > branch. 0.9.1 broker will return 0 as min supported version for the api
> > and
> > > 2 for the max supported version for the api. However, the version 1
> > should
> > > be clearly marked as deprecated on its documentation. It will be
> client's
> > > responsibility to make sure they are not using any such deprecated
> > version
> > > to the best knowledge of the client at the time of development (or
> > > alternatively by configuration)."
> > >
> > > Ismael
> > >
> > >
> > >
> > > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
> > >
> > > > A couple of questions:
> > > >
> > > > 1. The KIP says "Specific version may be deprecated through protocol
> > > > documentation but must still be supported (although it is fair to
> > return
> > > an
> > > > error code if the specific API supports it).". It may be worth
> > expanding
> > > > this a little more. For example, what does it mean to support the
> API?
> > I
> > > > guess this means that the broker must not disconnect the client and
> the
> > > > broker must return a valid protocol response. Given that it says that
> > it
> > > is
> > > > "fair" (I would probably replace "fair" with "valid") to return an
> > error
> > > > code if the specific API supports it, it sounds like we are saying
> that
> > > we
> > > > don't have to maintain the semantic behaviour (i.e. we could _always_
> > > > return an error for a deprecated API?). Is this true?
> > > >
> > > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> > ApiVersionRequest?
> > > >
> > > > Ismael
> > > >
> > >
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
> >
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Jun Rao <ju...@confluent.io>.
Magnus,
A while back, we had another proposal for the broker to just send the
correlation id and an empty payload if it receives an unsupported version
of the request. I didn't see that in the rejected section. It seems simpler
than the current proposal where the client has to issue an
ApiVersionRequest on every connection. Could you document the reason why we
rejected it?
Thanks,
Jun
On Tue, Apr 5, 2016 at 1:47 PM, Ashish Singh <as...@cloudera.com> wrote:
> On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Two more things:
> >
> > 3. We talk about backporting of new request versions to stable branches
> in
> > the KIP. In practice, we can't do that until the Java client is changed
> so
> > that it doesn't blindly use the latest protocol version. Otherwise, if
> new
> > request versions were added to 0.9.0.2, the client would break when
> talking
> > to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
> > gracefully, but that's still not good enough for a stable branch). It may
> > be worth making this clear in the KIP (yes, it is a bit orthogonal and
> > doesn't prevent the KIP from being adopted, but good to avoid confusion).
> >
> Good point. Adding this note and also adding a note that Kafka has not
> backported an api version so far.
>
> >
> > 4. The paragraph below is a bit confusing. It starts talking about 0.9.0
> > and trunk and then switches to 0.9.1. Is that intentional?
> >
> Yes.
>
> >
> > "Deprecation of a protocol version will be done by marking a protocol
> > version as deprecated in protocol documentation. Documentation shall also
> > be used to indicate a protocol version that must not be used, or for any
> > such information.For instance, say 0.9.0 had protocol versions [0] for
> api
> > key 1. On trunk, version 1 of the api key was added. Users running off
> > trunk started using version 1 of the api and found out a major bug. To
> > rectify that version 2 of the api is added to trunk. For some reason, it
> is
> > now deemed important to have version 2 of the api in 0.9.1 as well. To do
> > so, version 1 and version 2 both of the api will be backported to the
> 0.9.1
> > branch. 0.9.1 broker will return 0 as min supported version for the api
> and
> > 2 for the max supported version for the api. However, the version 1
> should
> > be clearly marked as deprecated on its documentation. It will be client's
> > responsibility to make sure they are not using any such deprecated
> version
> > to the best knowledge of the client at the time of development (or
> > alternatively by configuration)."
> >
> > Ismael
> >
> >
> >
> > On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > A couple of questions:
> > >
> > > 1. The KIP says "Specific version may be deprecated through protocol
> > > documentation but must still be supported (although it is fair to
> return
> > an
> > > error code if the specific API supports it).". It may be worth
> expanding
> > > this a little more. For example, what does it mean to support the API?
> I
> > > guess this means that the broker must not disconnect the client and the
> > > broker must return a valid protocol response. Given that it says that
> it
> > is
> > > "fair" (I would probably replace "fair" with "valid") to return an
> error
> > > code if the specific API supports it, it sounds like we are saying that
> > we
> > > don't have to maintain the semantic behaviour (i.e. we could _always_
> > > return an error for a deprecated API?). Is this true?
> > >
> > > 2. ApiVersionQueryRequest seems a bit verbose, why not
> ApiVersionRequest?
> > >
> > > Ismael
> > >
> >
>
>
>
> --
>
> Regards,
> Ashish
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ashish Singh <as...@cloudera.com>.
On Fri, Apr 1, 2016 at 1:32 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Two more things:
>
> 3. We talk about backporting of new request versions to stable branches in
> the KIP. In practice, we can't do that until the Java client is changed so
> that it doesn't blindly use the latest protocol version. Otherwise, if new
> request versions were added to 0.9.0.2, the client would break when talking
> to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
> gracefully, but that's still not good enough for a stable branch). It may
> be worth making this clear in the KIP (yes, it is a bit orthogonal and
> doesn't prevent the KIP from being adopted, but good to avoid confusion).
>
Good point. Adding this note and also adding a note that Kafka has not
backported an api version so far.
>
> 4. The paragraph below is a bit confusing. It starts talking about 0.9.0
> and trunk and then switches to 0.9.1. Is that intentional?
>
Yes.
>
> "Deprecation of a protocol version will be done by marking a protocol
> version as deprecated in protocol documentation. Documentation shall also
> be used to indicate a protocol version that must not be used, or for any
> such information.For instance, say 0.9.0 had protocol versions [0] for api
> key 1. On trunk, version 1 of the api key was added. Users running off
> trunk started using version 1 of the api and found out a major bug. To
> rectify that version 2 of the api is added to trunk. For some reason, it is
> now deemed important to have version 2 of the api in 0.9.1 as well. To do
> so, version 1 and version 2 both of the api will be backported to the 0.9.1
> branch. 0.9.1 broker will return 0 as min supported version for the api and
> 2 for the max supported version for the api. However, the version 1 should
> be clearly marked as deprecated on its documentation. It will be client's
> responsibility to make sure they are not using any such deprecated version
> to the best knowledge of the client at the time of development (or
> alternatively by configuration)."
>
> Ismael
>
>
>
> On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > A couple of questions:
> >
> > 1. The KIP says "Specific version may be deprecated through protocol
> > documentation but must still be supported (although it is fair to return
> an
> > error code if the specific API supports it).". It may be worth expanding
> > this a little more. For example, what does it mean to support the API? I
> > guess this means that the broker must not disconnect the client and the
> > broker must return a valid protocol response. Given that it says that it
> is
> > "fair" (I would probably replace "fair" with "valid") to return an error
> > code if the specific API supports it, it sounds like we are saying that
> we
> > don't have to maintain the semantic behaviour (i.e. we could _always_
> > return an error for a deprecated API?). Is this true?
> >
> > 2. ApiVersionQueryRequest seems a bit verbose, why not ApiVersionRequest?
> >
> > Ismael
> >
>
--
Regards,
Ashish
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ismael Juma <is...@juma.me.uk>.
Two more things:
3. We talk about backporting of new request versions to stable branches in
the KIP. In practice, we can't do that until the Java client is changed so
that it doesn't blindly use the latest protocol version. Otherwise, if new
request versions were added to 0.9.0.2, the client would break when talking
to a 0.9.0.1 broker (given Jason's proposal, it would fail a bit more
gracefully, but that's still not good enough for a stable branch). It may
be worth making this clear in the KIP (yes, it is a bit orthogonal and
doesn't prevent the KIP from being adopted, but good to avoid confusion).
4. The paragraph below is a bit confusing. It starts talking about 0.9.0
and trunk and then switches to 0.9.1. Is that intentional?
"Deprecation of a protocol version will be done by marking a protocol
version as deprecated in protocol documentation. Documentation shall also
be used to indicate a protocol version that must not be used, or for any
such information.For instance, say 0.9.0 had protocol versions [0] for api
key 1. On trunk, version 1 of the api key was added. Users running off
trunk started using version 1 of the api and found out a major bug. To
rectify that version 2 of the api is added to trunk. For some reason, it is
now deemed important to have version 2 of the api in 0.9.1 as well. To do
so, version 1 and version 2 both of the api will be backported to the 0.9.1
branch. 0.9.1 broker will return 0 as min supported version for the api and
2 for the max supported version for the api. However, the version 1 should
be clearly marked as deprecated on its documentation. It will be client's
responsibility to make sure they are not using any such deprecated version
to the best knowledge of the client at the time of development (or
alternatively by configuration)."
Ismael
On Fri, Apr 1, 2016 at 9:22 AM, Ismael Juma <is...@juma.me.uk> wrote:
> A couple of questions:
>
> 1. The KIP says "Specific version may be deprecated through protocol
> documentation but must still be supported (although it is fair to return an
> error code if the specific API supports it).". It may be worth expanding
> this a little more. For example, what does it mean to support the API? I
> guess this means that the broker must not disconnect the client and the
> broker must return a valid protocol response. Given that it says that it is
> "fair" (I would probably replace "fair" with "valid") to return an error
> code if the specific API supports it, it sounds like we are saying that we
> don't have to maintain the semantic behaviour (i.e. we could _always_
> return an error for a deprecated API?). Is this true?
>
> 2. ApiVersionQueryRequest seems a bit verbose, why not ApiVersionRequest?
>
> Ismael
>
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ismael Juma <is...@juma.me.uk>.
A couple of questions:
1. The KIP says "Specific version may be deprecated through protocol
documentation but must still be supported (although it is fair to return an
error code if the specific API supports it).". It may be worth expanding
this a little more. For example, what does it mean to support the API? I
guess this means that the broker must not disconnect the client and the
broker must return a valid protocol response. Given that it says that it is
"fair" (I would probably replace "fair" with "valid") to return an error
code if the specific API supports it, it sounds like we are saying that we
don't have to maintain the semantic behaviour (i.e. we could _always_
return an error for a deprecated API?). Is this true?
2. ApiVersionQueryRequest seems a bit verbose, why not ApiVersionRequest?
Ismael
Re: [DISCUSS] KIP-35 - Retrieve protocol version
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
I agree in a "perfect is the enemy of good" sense. I don't think what's
expressed in the API is necessarily the *ideal* thing for client
developers, which is who I think this API should be optimizing for. That
said, this is a practical solution to the problem and while finer-grained
than I think is actually needed, it's better to be too fine grained and
require a bit of extra effort for client developers than to be too
coarse-grained (e.g., global version numbers) and make it
difficult/impossible to get the desired behavior.
Short version: if Magnus and Dana think this is a reasonable solution, I'll
sign off on it since I see nothing glaringly wrong or bad about it and it
solves real problems for them :)
I think we arrived at a policy for Java client compatibility which works
fine when everything is developed in lockstep in the same codebase, but
isn't actually practical with other clients. In practice, no client
developers follow the same compatibility matrix -- they want clients
compiled with versions more recently than brokers to continue to work with
brokers. Although I understand how you can make the approach used with Java
clients work, I also understand why folks would want this other approach to
work. Given that a lot of clients want a different compatibility matrix
than the Java clients support and this KIP will enable that (even if it
requires some translations in the client libraries to figure out what
features are supported based on API versions), I think we should adopt this
support.
-Ewen
On Thu, Mar 31, 2016 at 3:05 PM, Ashish Singh <as...@cloudera.com> wrote:
> I agree with Jason. The blocking factor has been how to use the proposed
> changes in java client without making it super complicated. With Jason's
> suggestion we can get past this blocker, while keeping the core of the
> proposal intact.
>
>
> On Thu, Mar 31, 2016 at 2:51 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > Bumping this thread. I talked with Ashish and Magnus about this KIP
> offline
> > and I'm gradually coming over. The new API actually stands by itself
> > outside of the discussion about whether the client should support
> backwards
> > compatibility or not. For the Java client, we could continue to support
> the
> > current compatibility approach in which the client supports only brokers
> > with the same version or greater. In that case, we would use this API
> only
> > to assert that the current API versions are all supported, and raise an
> > exception if they are not. This gives us the capability going forward to
> > detect when the client is talking to an older broker, which we don't have
> > right now. This check should be straightforward, so we could do it now,
> > which would resolve some of the uneasiness about having an unused feature
> > which we depended on other clients to test for us. Does that make sense
> or
> > not?
> >
> > -Jason
> >
> > On Thu, Mar 17, 2016 at 4:06 PM, Ashish Singh <as...@cloudera.com>
> wrote:
> >
> > > We have proposed and discussed majorly three approaches so far, there
> > were
> > > many minor versions with small variations. Comparing them really
> > requires a
> > > side by side proposal and their pros/cons, and I agree with others that
> > > this has been lacking in the KIP. We just updated the KIP with
> following
> > > details.
> > >
> > > 1. Provide proposed changes in all the three proposals we have
> discussed
> > so
> > > far. Except the current proposal, these proposals are in rejected
> > > alternatives.
> > > 2. Provide reasoning on why the rejected proposals were rejected.
> > > 3. Add scenarios for all of these proposals from a client developer and
> > > core Kafka developer point of view.
> > >
> > > As we are really close to 0.10 deadline, a quick round of voting will
> > > really help. If you really do not like the idea, please feel free to
> say
> > > so. If the vote fails for the current proposal, it can at lease provide
> > > recommendations that we should consider for next version of proposal
> and
> > > put it up for vote again for next release. However, as stated earlier
> by
> > > multiple people having this ASAP will be awesome.
> > >
> > > On Thu, Mar 17, 2016 at 3:29 PM, Dana Powers <da...@gmail.com>
> > > wrote:
> > >
> > > > On Thu, Mar 17, 2016 at 1:42 PM, Gwen Shapira <gw...@confluent.io>
> > wrote:
> > > >
> > > > > "I think I would make this approach work by looking at the released
> > > > server
> > > > > version documentation for each version that I am trying to support
> > and
> > > > test
> > > > > against*, manually identify the expected "protocol vectors" each
> > > > supports,
> > > > > store that as a map of vectors to "broker versions", check each
> > vector
> > > at
> > > > > runtime until I find a match, and write code compatibility checks
> > from
> > > > > there."
> > > > >
> > > > > How is this better than a global version ID?
> > > >
> > > >
> > > > As a client developer, it seems roughly the same. I think it probably
> > > > avoids the server development workflow issues, and possibly the need
> to
> > > > agree on semantics of the global version ID? But others surely are
> more
> > > > qualified than me to comment on that part.
> > > >
> > > > -Dana
> > > >
> > >
> > >
> > >
> > > --
> > >
> > > Regards,
> > > Ashish
> > >
> >
>
>
>
> --
>
> Regards,
> Ashish
>
--
Thanks,
Ewen