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