-Re: Review Request 17248: Patch for KAFKA-1215
Jun Rao 2014-02-24, 17:55
This is an automatically generated e-mail. To reply, visit:
Sorry for the late review. The high level comment is that we need to make this change backward compatible, which means that we can do a rolling upgrade of an existing 0.8 cluster. To do that, we need to (1) consider wire protocol changes (see comments on Broker); (2) make sure that the old code can read the new ZK format. Once the following comments are addressed, could you exercise a rolling upgrade and see if there is any issue?
not => Not
Shouldn't we pass in maxReplicaPerRack?
After you rebase, we need to make sure this option is only available during topic creation.
broker list is for the clients and there is no need for them to know the rack a broker is in. host/port is enough.
Could we preserve the current constructor w/o rack and default rack to 0?
This actually changes the wire protocol btw the controller and the broker, which makes rolling upgrade difficult. If we really want to change the protocol, we need to do this in 2 steps. First upgrade the code w/o using the new format. Second, use the new format. So, we will need a new config option. For this particular case, I am not sure that we need to change the protocol though. We can only populate the rack info when instantiating a broker from ZK. This will make sure that we have the rack info for replica assignment. Then, we don't change the wire protocol btw the controller and the broker. All brokers instantiated through ByteBuffer will have the default rack info.
Could this be integrated into getCluster()?
- Jun Rao
On Feb. 1, 2014, 12:20 a.m., Joris Van Remoortere wrote: