Jun, I'm not sure we should default wire.protocol.version to the previous version. This will make fresh installs a bit weird :) I think we should default to the new version and assume that when I'm upgrading a broker, I'm re-using an existing configuration file.
This way, if I'm upgrading 0.8.3.0 to 0.9.0.0, the configuration file already says "wire.protocol.version=0.8.3.0" and I need to bump it post upgrade. Fresh install will include 0.9.0.0, so I won't need to bump anything. The only exception is with 0.8.2.0, where I'll need to add "wire.protocol.version=0.8.2.0" before upgrading to 0.8.3.0. Does that make sense? Regarding the naming, I agree that this parameter only controls the protocol between brokers (clients control the version of the protocol when they are involved, on a per-message basis). However, inter.broker.wire.protocol.version makes it sound like there may be other types of wire.protocol.version in the future, and I'm pretty sure we want a single parameter for controlling protocol versions from broker side. Not a big deal for me either way. On Wed, Feb 11, 2015 at 9:40 AM, Jun Rao <j...@confluent.io> wrote: > +1 for proposed changes in 1 and 2. > > 1. The impact is that if someone uses SimpleConsumer and references Broker > explicitly, the application needs code change to compile with 0.8.3. Since > SimpleConsumer is not widely used, breaking the API in SimpleConsumer but > maintaining overall code cleanness seems to be a better tradeoff. > > 2. For clarification, the issue is the following. In 0.8.3, we will be > evolving the wire protocol of UpdateMedataRequest (to send info about > endpoints for different security protocols). Since this is used in > intra-cluster communication, we need to do the upgrade in two steps. The > idea is that in 0.8.3, we will default wire.protocol.version to 0.8.2. When > upgrading to 0.8.3, in step 1, we do a rolling upgrade to 0.8.3. After step > 1, all brokers will be capable for processing the new protocol in 0.8.3, > but without actually using it. In step 2, we > configure wire.protocol.version to 0.8.3 in each broker and do another > rolling restart. After step 2, all brokers will start using the new > protocol in 0.8.3. Let's say that in the next release 0.9, we are changing > the intra-cluster wire protocol again. We will do the similar thing: > defaulting wire.protocol.version to 0.8.3 in 0.9 so that people can upgrade > from 0.8.3 to 0.9 in two steps. For people who want to upgrade from 0.8.2 > to 0.9 directly, they will have to configure wire.protocol.version to 0.8.2 > first and then do the two-step upgrade to 0.9. > > Gwen, > > In KIP2, there is still a reference to use.new.protocol. This needs to be > removed. Also, would it be better to use > intra.cluster.wire.protocol.version > since this only applies to the wire protocol among brokers? > > Others, > > The patch in KAFKA-1809 is almost ready. It would be good to wrap up the > discussion on KIP2 soon. So, if you haven't looked at this KIP, please take > a look and send your comments. > > Thanks, > > Jun > > > On Mon, Jan 26, 2015 at 8:02 PM, Gwen Shapira <gshap...@cloudera.com> > wrote: > > > Hi Kafka Devs, > > > > While reviewing the patch for KAFKA-1809, we came across two questions > > that we are interested in hearing the community out on. > > > > 1. This patch changes the Broker class and adds a new class > > BrokerEndPoint that behaves like the previous broker. > > > > While technically kafka.cluster.Broker is not part of the public API, > > it is returned by javaapi, used with the SimpleConsumer. > > > > Getting replicas from PartitionMetadata will now return BrokerEndPoint > > instead of Broker. All method calls remain the same, but since we > > return a new type, we break the API. > > > > Note that this breakage does not prevent upgrades - existing > > SimpleConsumers will continue working (because we are > > wire-compatible). > > The only thing that won't work is building SimpleConsumers with > > dependency on Kafka versions higher than 0.8.2. Arguably, we don't > > want anyone to do it anyway :) > > > > So: > > Do we state that the highest release on which SimpleConsumers can > > depend is 0.8.2? Or shall we keep Broker as is and create an > > UberBroker which will contain multiple brokers as its endpoints? > > > > 2. > > The KIP suggests "use.new.wire.protocol" configuration to decide which > > protocols the brokers will use to talk to each other. The problem is > > that after the next upgrade, the wire protocol is no longer new, so > > we'll have to reset it to false for the following upgrade, then change > > to true again... and upgrading more than a single version will be > > impossible. > > Bad idea :) > > > > As an alternative, we can have a property for each version and set one > > of them to true. Or (simple, I think) have "wire.protocol.version" > > property and accept version numbers (0.8.2, 0.8.3, 0.9) as values. > > > > Please share your thoughts :) > > > > Gwen > > >