I hope it is OK for me to make some progress here. I have made the following changes.
1. Updated KIP-35, to adopt Jay's suggestion on maintaining separate list of deprecated versions, instead of using a version of -1. 2. Added information on required permissions, Describe action on Cluster resource, to be able to retrieve protocol versions from a auth enabled Kafka cluster. Created https://issues.apache.org/jira/browse/KAFKA-3304. Primary patch is available to review, https://github.com/apache/kafka/pull/986 On Thu, Feb 25, 2016 at 1:27 PM, Ashish Singh <asi...@cloudera.com> wrote: > Kafka clients in Hadoop ecosystem, Flume, Spark, etc, have found it really > difficult to cope up with Kafka releases as they want to support users on > different Kafka versions. Capability to retrieve protocol version will go a > long way to ease out those pain points. I will be happy to help out with > the work on this KIP. @Magnus, thanks for driving this, is it OK if I carry > forward the work from here. It will be ideal to have this in 0.10.0.0. > > On Mon, Oct 12, 2015 at 9:29 PM, Jay Kreps <j...@confluent.io> wrote: > >> I wonder if we need to solve the error problem? I think this KIP gives a >> descent work around. >> >> Probably we should have included an error in the response header, but we >> debated it at the time decided not to and now it is pretty hard to add >> because the headers aren't versioned (d'oh). >> >> It seems like any other solution is going to be kind of a hack, right? >> Sending malformed responses back seems like not a clean solution... >> >> (Not sure if I was pro- having a top-level error or not, but in any case >> the rationale for the decision was that so many of the requests were >> per-partition or per-topic or whatever and hence fail or succeed at that >> level and this makes it hard to know what the right top-level error code >> is >> and hard for the client to figure out what to do with the top level error >> if some of the partitions succeed but there is a top-level error). >> >> I think actually this new API actually gives a way to handle this >> gracefully on the client side by just having clients that want to be >> graceful check for support for their version. Clients that do that will >> have a graceful message. >> >> At some point if we're ever reworking the headers we should really >> consider >> (a) versioning them and (b) adding a top-level error code in the response. >> But given this would be a big breaking change and this is really just to >> give a nicer error message seems like it probably isn't worth it to try to >> do something now. >> >> -Jay >> >> >> >> On Mon, Oct 12, 2015 at 8:11 PM, Jiangjie Qin <j...@linkedin.com.invalid> >> wrote: >> >> > I am thinking instead of returning an empty response, it would be >> better to >> > return an explicit UnsupportedVersionException code. >> > >> > Today KafkaApis handles the error in the following way: >> > 1. For requests/responses using old Scala classes, KafkaApis uses >> > RequestOrResponse.handleError() to return an error response. >> > 2. For requests/response using Java classes (only JoinGroupRequest and >> > Heartbeat now), KafkaApis calls AbstractRequest.getErrorResponse() to >> > return an error response. >> > >> > In KAFKA-2512, I am returning an UnsupportedVersionException for case >> [1] >> > when see an unsupported version. This will put the error code per topic >> or >> > partition for most of the requests, but might not work all the time. >> e.g. >> > TopicMetadataRequest with an empty topic set. >> > >> > Case [2] does not quite work for unsupported version, because we will >> > thrown an uncaught exception when version is not recognized (BTW this >> is a >> > bug). Part of the reason is that for some response types, error code is >> not >> > part of the response level field. >> > >> > Maybe it worth checking how each response is dealing with error code >> today. >> > A scan of the response formats gives the following result: >> > 1. TopicMetadataResponse - per topic error code, does not work when the >> > topic set is empty in the request. >> > 2. ProduceResonse - per partition error code. >> > 3. OffsetCommitResponse - per partition. >> > 4. OffsetFetchResponse - per partition. >> > 5. OffsetResponse - per partition. >> > 6. FetchResponse - per partition >> > 7. ConsumerMetadataResponse - response level >> > 8. ControlledShutdownResponse - response level >> > 9. JoinGroupResponse - response level >> > 10. HearbeatResponse - response level >> > 11. LeaderAndIsrResponse - response level >> > 12. StopReplicaResponse - response level >> > 13. UpdateMetadataResponse - response level >> > >> > So from the list above it looks for each response we are actually able >> to >> > return an error code, as long as we make sure the topic or partition >> won't >> > be empty when the error code is at topic or partition level. Luckily in >> the >> > above list we only need to worry about TopicMetadataResponse. >> > >> > Maybe error handling is out of the scope of this KIP, but I prefer we >> think >> > through how to deal with error code for the requests, because there are >> > more request types to be added in KAFKA-2464 and future patches. >> > >> > Thanks, >> > >> > Jiangjie (Becket) Qin >> > >> > On Mon, Oct 12, 2015 at 6:04 PM, Jay Kreps <j...@confluent.io> wrote: >> > >> > > Two quick pieces of feedback: >> > > 1. The use of a version of -1 as magical entry dividing between >> > deprecated >> > > versions is a bit hacky. What about instead having an array of >> supported >> > > versions and a separate array of deprecated versions. The deprecated >> > > versions would always be a subset of the supported versions. Or, >> > > alternately, since deprecation has no functional impact and is just a >> > > message to developers, we could just leave it out of the protocol and >> > just >> > > have it in release notes etc. >> > > 2. I think including the api name may cause some problems. Currently >> the >> > > api key is the primary key that we keep consistent but we have >> actually >> > > evolved the english description of the apis as they have changed. The >> > only >> > > use I can think of for the name would be if people used the logical >> name >> > > and tried to resolve the api key, but that would be wrong. Not sure >> if we >> > > actually need the english name, if there is a use case I guess we'll >> just >> > > have to be very clear that the name is just documentation and can >> change >> > > any time. >> > > >> > > -Jay >> > > >> > > On Fri, Sep 25, 2015 at 2:53 PM, Magnus Edenhill <mag...@edenhill.se> >> > > wrote: >> > > >> > > > Good evening, >> > > > >> > > > KIP-35 was created to address current and future broker-client >> > > > compatibility. >> > > > >> > > > >> > > > >> > > >> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-35+-+Retrieving+protocol+version >> > > > >> > > > Summary: >> > > > * allow clients to retrieve the broker's protocol version >> > > > * make broker handle unknown protocol requests gracefully >> > > > >> > > > Feedback and comments welcome! >> > > > >> > > > Regards, >> > > > Magnus >> > > > >> > > >> > >> > > > > -- > > Regards, > Ashish > -- Regards, Ashish
