Hello Gwen/ Ismael, On Mon, Mar 14, 2016 at 9:55 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> Hi Gwen, > > On Mon, Mar 14, 2016 at 4:37 PM, Gwen Shapira <g...@confluent.io> wrote: > > > On Mon, Mar 14, 2016 at 7:05 AM, Ismael Juma <ism...@juma.me.uk> wrote:> > > >> 1. Every time a protocol version changes, for any request/response, > > >> broker version, ApiVersion, will be bumped up. > > >> > > > > > > Any thoughts on how we will enforce this? > > > > Code reviews? :) > > > > We are already doing it in ApiVersion (and have been since > > 0.8.2.0-SNAPSHOT). Enforcing is awesome, but not necessarily part of > > this KIP. > > > > What we have been doing since 0.8.2.0-SNAPSHOT is to create a new > `ApiVersion` for each new release. What is being proposed here is to create > a new `ApiVersion` every time a protocol version changes for any > request/response. This is much easier to miss. Admittedly, this approach > was introduced as part of KIP-31/32, but if we are going to expose this > version to clients, I think it is good to think about ways to ensure > correctness. It may be that we decide that it's out of scope or that we can > do it later, but I don't think we should just dismiss it without even > thinking about it. > I think ApiVersion aims to identify protocol version changes, more than release change, https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/api/ApiVersion.scala#L30. I do agree that having an automated check to ensure this happens will be really useful. > > >> 4. On getting unknown protocol version, broker will send an empty > > >> response, instead of simply closing client connection. > > >> > > > > > > I am not sure about this one. It's an unusual pattern and feels like a > > hack. > > > > We discussed this and failed to come up with a better solution that > > doesn't break compatibility. > > Improvements can be added in the future - nothing can be worse than > > current state (where the broker silently closes the connection) > > > > My understanding is that this doesn't help clients that support KIP-35 > since they will know the broker version. And for older clients, they will > fail with a parsing exception, which is a bit better, but not much better. > So, is it really worth doing? In the KIP call we had about this months ago, > there was no consensus on this one from what I remember. > That is a good point! However, what about a client that wants to support broker versions that do not provide broker version in metadata and broker versions that provides version info in metadata. I think having this does not cost us anything, but enables such clients to be smart. > > Ismael > -- Regards, Ashish
