2015-09-28 20:59 GMT+02:00 Jun Rao <j...@confluent.io>: > 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? >
My idea with this is that upon initial connect to a broker the client would send a BrokerMetadataRequest to understand what the broker supports (protocol versions, codecs, message format versions, config values, etc), and as such the information in the response should not change during the lifetime of a connection. So I dont think we want to put volatile stats-like data in there, we should use specific APIs for that when the need arise. > As for sending an empty response for unrecognized requests, do you how is > that handled in other similar systems? > Not sure how this applies to other systems, but this workaround is to overcome the lack of a generic request-wide error field in the common protocol header. While it is a bit of a hack to trigger a parsing error on the client, that parsing error will be in the context of the failed request, which will hopefully(!) emit an at least somewhat useful error message to the application/user. > > 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 > > > > > >
