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 > > >
