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. 101. We already use varInt in the message format. I assume that the protocol uses the same varInt representation? 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 > > > > > > > > > > > > > > >
