Nice write-up. Just had a question, instead of returning an empty response back to the client, would it be better for the broker to return a response that gives some more info to the client regarding the min version they need to upgrade to in order to communicate with the broker.
Thanks, Mayuresh On Mon, Sep 28, 2015 at 6:36 PM, Jiangjie Qin <j...@linkedin.com.invalid> wrote: > Thanks for the writeup. I also think having a specific protocol for > client-broker version negotiation is better. > > I'm wondering is it better to let the broker to decide the version to use? > It might have some value If brokers have preference for a particular > version. > Using a global version is a good approach. For the client-broker > negotiation, I am thinking about something like: > > ProtocolSyncRequest => ClientType [ProtocolVersion] > ClientType => int8 > ProtocolVersion => int16 > > ProtocolSyncResponse => PreferredVersion > PreferredVersion => int16 > > Thanks, > > Jiangjie (Becket) Qin > > On Mon, Sep 28, 2015 at 11:59 AM, Jun Rao <j...@confluent.io> wrote: > > > I agree with Ewen that having the keys explicitly specified in the > response > > is better. > > > > In addition to the supported protocol version, there are other > interesting > > metadata at the broker level that could be of interest to things like > admin > > tools (e.g., used disk space, remaining disk space, etc). I am wondering > if > > we should separate those into different requests. For inquiring the > > protocol version, we can have a separate BrokerProtocolRequest. The > > response will just include the broker version and perhaps a list of > > supported requests and versions? > > > > As for sending an empty response for unrecognized requests, do you how is > > that handled in other similar systems? > > > > Thanks, > > > > Jun > > > > On Mon, Sep 28, 2015 at 10:47 AM, Jason Gustafson <ja...@confluent.io> > > wrote: > > > > > Having the version API can make clients more robust, so I'm in favor. > One > > > note on the addition of the "rack" field. Since this is a > broker-specific > > > setting, the client would have to query BrokerMetadata for every new > > broker > > > it connects to (unless we also expose rack in TopicMetadata). This is > > also > > > kind of unfortunate for admin utilities leveraging this API. It might > be > > > more convenient to allow this API to return broker metadata for the > full > > > cluster, assuming all of it could be made available in Zookeeper. > > > > > > As for using the empty response to indicate an incompatible API, it > seems > > > like that could work. I think some of the clients might catch response > > > parsing exceptions and retry anyway, but that's no worse than retrying > > > because of a disconnect in the same case. > > > > > > -Jason > > > > > > On Fri, Sep 25, 2015 at 9:34 PM, Ewen Cheslack-Postava < > > e...@confluent.io> > > > wrote: > > > > > > > The basic functionality is definitely useful here. I'm generally in > > favor > > > > of exposing some info about broker versions to clients. > > > > > > > > I'd prefer to keep the key/values explicit. Making them extensible > > > > string/string pairs is good for avoiding unnecessary version changes > in > > > the > > > > protocol, but I think we should explicitly define the valid key/value > > > > formats in each version of the protocol. New keys can safely be > > ignored, > > > > but actually specifying the format of the values is important if we > > ever > > > > need to evolve those formats. > > > > > > > > I like some of the examples you've provided for returned key/value > > pairs > > > > and I think we should provide some of them even when the values > should > > be > > > > obvious from the broker version. > > > > > > > > * broker.version - are we definitely standardizing on this versioning > > > > format? 4 digits, with each level indicating the intuitive levels of > > > > compatibility? Is there any chance we'll have a 0.10.0.0? This might > > seem > > > > like a trivial consideration, but after fighting versioning in > > different > > > > packaging systems, I'm a bit more sensitive to the annoying effects > > that > > > > not considering this carefully can have. We're at a particularly > > > sensitive > > > > point as we hit .9 -> .10 or .9 -> 1.0 (!!!!). > > > > * supported.compression.codecs - nit, but I'd like to figure out a > good > > > way > > > > to keep these as close to the actual config name as possible. in this > > > case, > > > > the setting is compression.codec, so just "compression.codecs" seems > > > ideal > > > > to me. > > > > * rack: I think there's a ton of demand for something like this, but > > I'd > > > > really like to think through it more before exposing anything. "rack" > > is > > > > *very* specific to a deployment scenario. I think we're comfortable > > > > adapting the terminology, but the meaning can change drastically, > even > > > > under seemingly similar deployments. For example, if you deploy in > EC2, > > > you > > > > might create instances in multiple AZs. Within an AZ, you might > > consider > > > > all the nodes on the same "rack". But there are also placement groups > > > > within each AZ that provide better guarantees on network performance. > > Are > > > > any nodes in the same AZ considered on the same rack or do you need > > > special > > > > guarantees to be on the same "rack". In general, I don't think > there's > > a > > > > generic "rack" identifier we can expose -- we'll need to do something > > > > specialized depending on different environments. This is a case where > > > > extensibility in the format is probably really useful. > > > > * Properties like supported.compression.codecs can presumably be > > > determined > > > > just via the broker version. Is there a good reason for including > them? > > > > Perhaps explicit info >> implicit info? > > > > * "All existing clients should be able to handle this gracefully > > without > > > > any alterations to the code" - which clients does this refer to? For > > > > example, will the Java clients continue to behave the same way they > do > > > > today? I'm curious because empty responses seem *very* different from > > > > closing connections. > > > > > > > > > > > > -Ewen > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > -- > > > > Thanks, > > > > Ewen > > > > > > > > > > -- -Regards, Mayuresh R. Gharat (862) 250-7125
