The description that Jun gave for (2) was the detail I was looking for - Gwen can you update the KIP with that for completeness/clarity?
I'm +1 as well overall. However, I think it would be good if we also get an ack from someone who is more experienced on the operations side (say, Todd) to review especially the upgrade plan. On Wed, Feb 11, 2015 at 09:40:50AM -0800, Jun Rao 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 > >
