On Mon, Mar 14, 2016 at 7:05 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Hi Ashish, > > A few comments below. > > On Fri, Mar 11, 2016 at 9:59 PM, Ashish Singh <asi...@cloudera.com> wrote: > >> Sounds like we are mostly in agreement. Following are the key points. >> >> 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. > > >> 2. Protocol documentation will be versioned with broker version. Every >> time there is a broker version change, protocol documentation version >> needs >> to be updated and linked to main documentation page. >> 3. Deprecation of protocol version will be done via marking the version >> as deprecated on the protocol documentation. >> > > I think this is fine for the first version. We may consider doing more in > the future (logging a warning perhaps). > > >> 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) > > 5. Metadata response will be enhanced to also contain broker version, >> VersionInt and VersionString. VersionString will contain internal >> version information. >> > > Even though Magnus suggested that it's OK for clients to parse > `VersionString`, I personally would rather avoid that. Do we really need 3 > separate versions or could we get away with 2? I think it would be good to > elaborate on this a bit and explain how each of these versions would be > used (both from the broker and clients perspective). Agree! I'm also confused. > >> 6. Metadata request with single null topic and size of -1 can be used to >> fetch metadata response with only broker version information and no >> topic/broker info. > > 7. On receiving a metadata request with single null topic with size of >> -1, broker will respond with only broker version. >> > > As Magnus says, the broker information should be returned. This would also > help us reduce unnecessary data transfer during NetworkClient's metadata > updates (KAFKA-3358). At the moment, we get information for all topics in > situations where we actually want no topics. > > Also, I think it's a bit odd to say a `single null topic with size -1`. Do > we mean an array of topics with size -1 and no elements? That would imply > introducing a NULLABLE_ARRAY type (we currently have NULLABLE_STRING and > NULLABLE_BYTES). > > Ismael
