On Fri, Sep 20, 2019, at 18:05, Jun Rao wrote: > Hi, Colin, > > Thanks for the KIP. Overall, looks good to me too. A couple of minor > comments. > > 100. Does the tag number need to be 31-bit? It seems that 15-bit could be > enough.
I think 15 bit would probably be enough, but there is no extra overhead to allowing 31 bits when using a variable-length integer. > > 101. We already use varInt in the message format. I assume that the > protocol uses the same varInt representation? It uses a slightly different varint representation. Basically, the difference is that the existing representation uses serpentine encoding to make representing negative numbers more efficient, at the cost of making positive numbers less efficient. Since tags (and lengths) can't be negative, there is no need for serpentine encoding, and we can be more efficient without it. best, Colin > > Jun > > > > > On Fri, Sep 6, 2019 at 3:06 PM Colin McCabe <cmcc...@apache.org> wrote: > > > Hi all, > > > > The KIP-482 vote has passed. Thanks to everyone who voted or discussed > > the issue. > > > > There were 3 non-binding +1 votes from David Jacot, Jose Armando Garcia > > Sancio, and Satish Duggana. > > There were 3 binding +1 votes from Harsha Chintalapani, David Arthur, and > > Jason Gustafson. > > > > best, > > Colin > > > > On Fri, Sep 6, 2019, at 13:36, Jason Gustafson wrote: > > > +1 Thanks Colin. This is really going to help with compatibility. > > > > > > -Jason > > > > > > On Wed, Sep 4, 2019 at 1:34 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > > > On Wed, Sep 4, 2019, at 13:01, Jason Gustafson wrote: > > > > > Hi Colin, > > > > > > > > > > Just a couple questions. > > > > > > > > > > 1. I think we discussed that we would do a lazy version bump of all > > > > > protocols in order to get flexible version support. Can you add that > > to > > > > the > > > > > KIP? > > > > > > > > Good point. Added. > > > > > > > > > 2. The doc mentions a bump to the request and response header > > formats to > > > > > version 1. Currently there is no formal header version. It wasn't > > clear > > > > to > > > > > me if you were suggesting that we create a header version as part of > > the > > > > > schema or if this was just an informal way to refer to the header in > > > > > "flexible version" requests. Can you clarify? > > > > > > > > I think we should have a formal header version. However, we can deduce > > > > which header version we should use based on the apiKey and apiVersion, > > so > > > > no changes will be needed to what is sent over the wire. > > > > > > > > Having a new header version will let us add new fields to request and > > > > response headers. In particular, having a flexible header version > > will let > > > > us add tagged fields, which will be useful for adding things like a > > message > > > > traceID. > > > > As another example, ThrottleTimeMs would have made more sense in the > > > > response header than as an addition to every message. > > > > > > > > cheers, > > > > Colin > > > > > > > > > > > > > > Thanks, > > > > > Jason > > > > > > > > > > On Wed, Sep 4, 2019 at 8:14 AM David Arthur <mum...@gmail.com> > > wrote: > > > > > > > > > > > +1 binding. > > > > > > > > > > > > Thanks for the KIP, Colin! > > > > > > > > > > > > -David > > > > > > > > > > > > On Wed, Sep 4, 2019 at 5:40 AM Harsha Chintalapani < > > ka...@harsha.io> > > > > > > wrote: > > > > > > > > > > > > > LGTM. +1 (binding) > > > > > > > -Harsha > > > > > > > > > > > > > > > > > > > > > On Wed, Sep 04, 2019 at 1:46 AM, Satish Duggana < > > > > > > satish.dugg...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > +1 (non-binding) Thanks for the nice KIP. > > > > > > > > > > > > > > > > You may want to update the KIP saying that optional tagged > > fields > > > > do > > > > > > not > > > > > > > > support complex types(or structs). > > > > > > > > > > > > > > > > On Wed, Sep 4, 2019 at 3:43 AM Jose Armando Garcia Sancio > > > > > > > > <jsan...@confluent.io> wrote: > > > > > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > > > > > Looking forward to this improvement. > > > > > > > > > > > > > > > > On Tue, Sep 3, 2019 at 12:49 PM David Jacot < > > dja...@confluent.io> > > > > > > wrote: > > > > > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > > > > > Thank for the KIP. Great addition to the Kafka protocol! > > > > > > > > > > > > > > > > Best, > > > > > > > > David > > > > > > > > > > > > > > > > Le mar. 3 sept. 2019 à 19:17, Colin McCabe <cmcc...@apache.org> > > a > > > > > > écrit > > > > > > > : > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I'd like to start the vote for KIP-482: The Kafka Protocol > > should > > > > > > Support > > > > > > > > Optional Tagged Fields. > > > > > > > > > > > > > > > > KIP: > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > > > > > > > > > KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields > > > > > > > > > > > > > > > > Discussion thread here: > > > > > > > > > > > > > > > > https://lists.apache.org/thread.html/ > > > > > > > > cdc801ae886491b73ef7efecac7ef81b24382f8b6b025899ee343f7a@ > > > > %3Cdev.kafka. > > > > > > > > apache.org%3E > > > > > > > > > > > > > > > > best, > > > > > > > > Colin > > > > > > > > > > > > > > > > -- > > > > > > > > -Jose > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > David Arthur > > > > > > > > > > > > > > > > > > > > >
