2015-09-29 17:25 GMT+02:00 Grant Henke <ghe...@cloudera.com>: > If we create a protocol version negotiation api for clients, can we use it > to replace or improve the ease of upgrades that break inter-broker > messaging? > > Currently upgrades that break the wire protocol take 2 rolling restarts. > The first restart we set inter.broker.protocol.version telling all brokers > to communicate on the old version, and then we restart again removing the > inter.broker.protocol.version. With this api the brokers could agree on a > version to communicate with, and when bounced re-negotiate to the new > version. >
Interesting point. I'm still not convinced we want a stateful ProtocolVersionSync handshake, it adds complexity, and since only one side of a Kafka connection is issuing requests only that side needs to decide what protocol version to use - that side is the client (or replica?). But yeah, BrokerMetadataRequest could be used in broker-broker communications too. > > On Mon, Sep 28, 2015 at 10:26 PM, Mayuresh Gharat < > gharatmayures...@gmail.com> wrote: > > > 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 > > > > > > -- > Grant Henke > Software Engineer | Cloudera > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke >
