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? > > > > 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. > > 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). > > > > 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). > Missed this point. I was thinking of an array with one null string, ArrayOf[NULLABLE_STRING]. However, we can add NullableArrayOf and use NullableArrayOf[STRING] as well. What do you think is better here? > > Ismael > -- Regards, Ashish
