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