NULLABLE_STRING was just committed to trunk: https://github.com/apache/kafka/pull/866
Also we should pull in this PR before making changes to UpdateMetadata: https://github.com/apache/kafka/pull/896 Thanks, Grant On Fri, Feb 12, 2016 at 8:16 PM, Joel Koshy <jjkosh...@gmail.com> wrote: > We are adding a NULLABLE_STRING type (KAFKA-3088) but you would then need > to evolve the UpdateMetadata request. Regardless, it seems better to just > go with an empty string. > > On Fri, Feb 12, 2016 at 5:38 PM, Allen Wang <allenxw...@gmail.com> wrote: > > > In implementing changes to UpdateMetadataRequest, I noticed > > that org.apache.kafka.common.protocol.types.STRING does not allow null > > value. This creates a problem for rack as it is an optional field for > > broker. In Scala, it is declared as Option[String]. I was planning to > > transmit the rack as null in the protocol if rack is not configured for > the > > broker. > > > > There are two options: > > > > - Transmit the rack as empty string if rack is not configured for the > > broker. This implies that empty string cannot be used for the rack we > need > > to do this validation. This is reasonable since empty string for the rack > > is most likely a user error and I cannot think of a use case why users > > would pick empty string as rack. It does create some inconsistency > between > > what gets transmitted on the wire vs. the actual value in broker runtime. > > > > - Change STRING to allow null. I think that is also reasonable since > > ApiUtils.writeShortString and ApiUtils.readShortString APIs support null. > > However, I would like to know if there is any particular reason not to > > allow null for STRING. > > > > Any opinions? > > > > Thanks, > > Allen > > > > > > On Wed, Jan 20, 2016 at 1:50 PM, Allen Wang <allenxw...@gmail.com> > wrote: > > > > > Hi Arun, > > > > > > This is about making replica assignment rack aware. It is not about > > making > > > replica assignment algorithm pluggable. I think plug-ability should be > > > discussed separately from this KIP. > > > > > > Thanks, > > > Allen > > > > > > > > > On Tue, Jan 19, 2016 at 11:16 PM, Arun Mahadevan <ar...@apache.org> > > wrote: > > > > > >> Nice feature. Is this going to support only rack aware assignments? > > >> > > >> May be nice to make the implementation pluggable (with rack aware > being > > >> one) so that other kind of assignment algorithms can be plugged in > > future. > > >> > > >> - Arun > > >> > > >> > > >> > > >> On 1/15/16, 12:22 AM, "Allen Wang" <allenxw...@gmail.com> wrote: > > >> > > >> >Thanks Ismael. KIP is updated to use 0.9.0.0 and add link to the > JIRA. > > >> > > > >> > > > >> >On Thu, Jan 14, 2016 at 8:46 AM, Ismael Juma <ism...@juma.me.uk> > > wrote: > > >> > > > >> >> On Thu, Jan 14, 2016 at 1:24 AM, Allen Wang <allenxw...@gmail.com> > > >> wrote: > > >> >> > > >> >> > Updated KIP regarding how broker JSON version will be handled and > > new > > >> >> > procedure of upgrade. > > >> >> > > >> >> > > >> >> Thanks Allen. In the following text, I think we should replace > 0.9.0 > > >> with > > >> >> 0.9.0.0: > > >> >> > > >> >> "Due to a bug introduced in 0.9.0 in ZkUtils.getBrokerInfo(), old > > >> clients > > >> >> will throw an exception when it sees the broker JSON version is > not 1 > > >> or 2. > > >> >> Therefore, *a minor release 0.9.0.1 is required* to fix the problem > > >> first > > >> >> so that old clients can parse future version of broker JSON in > > >> ZooKeeper. > > >> >> That means 0.9.0 clients must be upgraded to 0.9.0.1 before 0.9.1 > > >> upgrade > > >> >> can start. In addition, since ZkUtils.getBrokerInfo() is also used > by > > >> >> broker, version specific code has to be used when registering > broker > > >> with > > >> >> ZooKeeper" > > >> >> > > >> >> Also, I posted a PR for supporting version > 2 in 0.9.0.1 and > trunk: > > >> >> > > >> >> https://github.com/apache/kafka/pull/773 > > >> >> > > >> >> Ismael > > >> >> > > >> > > >> > > > > > > -- Grant Henke Software Engineer | Cloudera gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
